[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S): 8153890: Handle unsafe access error as an asynchronous exception
From:       Mikael Vidstedt <mikael.vidstedt () oracle ! com>
Date:       2016-04-26 17:49:02
Message-ID: 571FAA0E.4050503 () oracle ! com
[Download RAW message or body]


I filed JDK-8154592 to cover handling the faulting accesses completely 
synchronously instead (much like NPEs), and I am withdrawing this RFR in 
favor of that.

Cheers,
Mikael

[1] https://bugs.openjdk.java.net/browse/JDK-8154592

On 4/11/2016 6:54 PM, David Holmes wrote:
> The light is starting to shine ... :)
>
> On 12/04/2016 10:35 AM, David Holmes wrote:
>> 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.
>
> Ah! So now I think I see what the real bug is - it isn't timeliness it 
> is a correctness issue. It should be impossible to return from native 
> to Java without converting a pending "async error" into an actual 
> exception. But the native call stub calls 
> JavaThread::check_special_condition_for_native_trans, which in turn 
> calls:
>
>  thread->check_and_handle_async_exceptions(false);
>
> which skips the unsafe-access-error check. And that is wrong if the 
> Unsafe methods are called as normal native methods. But we can't do 
> the unsafe-access-error check while we are in a "trans" state if it 
> will allocate.
>
> The existing code seems to assume that unsafe-access will not occur 
> via the normal native call path. Exactly how are these unsafe accesses 
> performed?
>
> David
> -----
>
>> 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
>>>

[prev in list] [next in list] [prev in thread] [next in thread]