RFR(s): 8228758: assert(_no_handle_mark_nesting == 0) failed: allocating handle inside NoHandleMark

[Robbin Ehn]

Hi Dan,

On 9/3/19 10:38 PM, Daniel D. Daugherty wrote:
>>
>> We already have that today with safepoint, many parts of subsystems can't handle
>> safepoints. The one adding the new code to be run in a handshake must make sure
>> that works. Not saying we should be able to run anything in a handshake, but
>> fundamental things like Handles should be allowed.
> 
> I'm going to have to disagree with the comparison here. I don't think
> comparing handshakes with blocking for safepoints is quite right. A
> thread blocking for a safepoint while executing a subsystem's code is
> a synchronous thing in my mind:
> 
>      I'm calling into the subsystem and that subsystem uses locks so my
>      thread might safepoint while in there...
> 
>      The subsystem has to synchronously prepare itself for grabbing that
>      lock by Handle-izing oops, etc...
> 
> When I think about handshakes, I think about them as happening asynchronously
> like async-signals resulting in calls to handlers... Maybe my mental model of
> handshakes is wrong... :-)

No your model also make sense.
Generically speaking, some functions are not allowed to be called from the 
signal handler (not re-entrant) and those that must be called from signal 
handler but are not re-entrant have to block signals while executing.

When you do a e.g. jvmti stack, if it's done in a safepoint or handshake have 
from the caller perspective no difference (asynch vs synch). But with a 
handshake we have contending threads, thus often require locking, which makes it 
more deadlock prone. I think having contending threads is the major difference.

> 
> In any case, I'm going to hold off on reviewing the code since I think
> you're still considering making changes based on David's last set of
> comments...

I think David is okay with current patch.

Thanks, Robbin

> 
> Dan
> 
> 
> 
>>
>>>
>>>> This changes the polling model a bit and we have not proper look at this.
>>>> Maybe we should add the constraints to the poll and to find any potential
>>>> troublesome polls.
>>>
>>> I'm not sure how we would discover the problems until we have a handshake 
>>> that executes some code that violates a constraint. The constraints 
>>> themselves are not obvious. Subsystems do not clearly identify themselves as 
>>> being reentrant or not - most things are not reentrant by their nature.
>>
>> As we do with safepoints, NoSafepointVerifier (since it covers both cases).
>>
>> /Robbin
>>
>>>
>>> Cheers,
>>> David
>>>
>>>> Thanks, Robbin
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>>
>>>>>> Thanks, Robbin
>>>>>>
>>>>>> Some extra words about NoHandleMark:
>>>>>> I believe NoHandleMark should be removed.
>>>>>> In the LEAF case, there is no point in creating a Handle within a
>>>>>> NoSafepointVerifier, since we don't safepoint.
>>>>>> The other usecase seems to be, I'm in java don't do VM things, and someone 
>>>>>> tried
>>>>>> to protected that with a NoHandleMark. There many ways to create a much more
>>>>>> robust check for that.
>>>>>> But a discussion for another time.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Issue:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8228758
>>>>>>>>
>>>>>>>> Changeset:
>>>>>>>> http://cr.openjdk.java.net/~rehn/8228758/webrev/
>>>>>>>>
>>>>>>>> Passes t1-3 (and performance benchmarks)
>>>>>>>>
>>>>>>>> Thanks, Robbin
>