RFR(S): 8153890: Handle unsafe access error as an asynchronous exception

[David Holmes]

Looking at the test ...

On 9/04/2016 8:27 AM, Mikael Vidstedt wrote:
>
> Please review:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8153890
> Webrev:
> http://cr.openjdk.java.net/~mikael/webrevs/8153890/webrev.01/hotspot/webrev/
>
>
> * Note: this is patch 1 in a set of 3 all aiming to clean up and unify
> the unsafe memory getters/setters, along with the handling of unsafe
> access errors. The other two issues are:
>
> https://bugs.openjdk.java.net/browse/JDK-8153892 - Handle unsafe access
> error directly in signal handler instead of going through a stub
> https://bugs.openjdk.java.net/browse/JDK-8150921 - Update Unsafe
> getters/setters to use double-register variants
>
>
> * Summary (copied from the bug description)
>
> In certain cases, such as accessing a region of a memory mapped file
> which has been truncated on unix-style operating systems, a SIGBUS
> signal will be raised and the VM will process it in the signal handler.
> Currently, as part of the handling a flag is set on the thread
> indicating that an error has occurred:
>
> _special_runtime_exit_condition = _async_unsafe_access_error;
>
> Eventually the VM will check the flag and throw an InternalError.
> However, the flag is not always checked promptly, which means that the
> time between the error and the time when the InternalError is thrown can
> be very long.
>
> In parallel with this, there is functionality in the VM to set pending
> asynchronous exceptions on threads. These asynchronous exceptions are
> checked/thrown more often.
>
> Instead of having two mechanisms for handling these types of
> asynchronous events the unsafe access errors should be converted to use
> the normal asynchronous exception mechanism.
>
> One of the reasons why this isn't already the case is that when the
> SIGBUS signal is raised, the thread may not be in a place where it can
> allocate an exception - there may not be a safepoint on the instruction
> performing the memory access. The solution to this is to preallocate the
> InternalError exception at VM start up, and have the handler code simply
> use that exception instead. The only downside of this is that the
> exception will not have a stack trace. The flip side of that is that
> because of the unpredictability in when the async unsafe access error is
> actually thrown today it is likely that the stack trace in the exception
> doesn't actually reflect the place where the error actually occurred
> anyway.
>
> * Testing
>
> In addition to the JPRT smoke tests a new test has been developed for
> testing unsafe access errors, in particular accessing data in the
> truncated portion of a memory mapped file using the Unsafe primitives.
> The test can be found here:
>
> http://cr.openjdk.java.net/~mikael/webrevs/8150921/MappedTruncated.java

Here:

        } else {
             r.run();

             // The operation may have raised a pending async
             // exception, make sure it's actually thrown
             forcePendingAsyncExceptionProcessing();
         }

forcePendingAsyncExceptionProcessing should not be needed. There is no 
way we should be able to return to Java without the pending exception 
being thrown from r.run()! If we can then there is a serious bug somewhere.

Thanks,
David

> The plan is to push it together with the final patch (JDK-8150921). Note
> though that the test fails unless the other patches mentioned above are
> also applied.
>
> I am also looking for to run additional testing on this before pushing,
> but wanted to get some early feedback.
>
> Cheers,
> Mikael
>