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

[Robbin Ehn]

Hi David,

Truncated:

> 
> You stated that if you need to not use any handleMarks then you most likely 
> don't want polls - and no polls is what a LEAF routine has to guarantee:
> 
> QUICK -> No handles -> no polls == LEAF

I'll look at the other way around:
QUICK can have polls therefore they cannot be leafs.
(at least the one triggering this assert)

> 
>> Since we missing the cleaner, accidentally using a HandleMark would lead to a 
>> leak. The NoHandleMark protected against this leak.
> 
> Maybe I'm misunderstanding exactly how HandleMarks nest and behave. I would have 
> thought this nesting would be allowed:
> 
> entry_point
>    NoHandleMark
>    -> subroutine
>       HandleMark  // prevents leak of Handle to outer scope
>       allocate handle
> 
> whereas this would cause an error
> 
> entry_point
>    NoHandleMark
>    -> subroutine
>       allocate handle
> 
> but IIUC a NoHandleMark prevents all and any handle use in its own scope or a 
> nest scope - no?

Yes, sorry, NoHandleMark increments _no_handle_mark_nesting and
HandleArea::allocate_handle checks that it is 0, otherwise this assert.
(it also checks that _handle_mark_nesting is larger than 0, so it must be inside 
a HandleMark)

> 
> I'm not sure that is viable, or the right way to try and address this. In the 
> extreme it would require all VM susbsystems to be fully re-entrant and that is 
> not a reasonable expectation.

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.

> 
>> 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