RFR: 8259713: Fix comments about ResetNoHandleMark in deoptimization

[David Holmes]

On Thu, 14 Jan 2021 01:46:13 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/runtime/deoptimization.cpp line 693:
>> 
>>> 691:   // Handles will be leaked.
>>> 692:   ResetNoHandleMark rnhm;
>>> 693:   HandleMark hm(thread);
>> 
>> Here's a simpler rewrite for L687 -> L693:
>> 
>>     // This code wants to allocate handles which is okay in a leaf method,
>>     // but JRT_LEAF contains a NoHandleMark so we have to work around
>>     // that with ResetNoHandleMark in order to create a HandleMark here:
>>     ResetNoHandleMark rnhm;
>>     HandleMark hm(thread);
>
> Unfortunately this is not entirely what I want to say.
> I want to say that you *must* have a HandleMark if you have ResetNoHandleMark in a JRT_LEAF.
> You need a ResetNoHandleMark to create Handles but there are no asserts to make sure you have a HandleMark.
> ie:
> 
>      ResetNoHandleMark rnhm;
>      Handle h(THREAD, obj);  // OK no problem
> 
> But the Handle will leak once this function returns (until the next JRT_ENTRY anyway).
> 
>     // This code wants to allocate handles which is okay in a leaf method,
>     // but JRT_LEAF contains a NoHandleMark so we have to work around
>     // that with ResetNoHandleMark in order to create a Handles here.
>     // There must also be a HandleMark here to clean up any handles created in this scope.
> 
> How about that?

My suggestion:
// JRT_LEAF methods don't normally allocate handles and there is a
// NoHandleMark to enforce that. It is actually safe to use Handles
// in a JRT_LEAF method, and sometimes desirable, but to do so we
// must use ResetNoHandleMark to bypass the NoHandleMark, and
// then use a HandleMark to ensure any Handles we do create are
// cleaned up in this scope.
I still question why such a function declares itself a LEAF instead of just being a JRT_ENTRY but that is a different argument. :)
Cheers,
David

-------------

PR: https://git.openjdk.java.net/jdk/pull/2069