[15] RFR(XS): 8244946: fatal error: memory leak: allocating without ResourceMark with -XX:+Verbose -Xlog:methodhandles

[coleen.phillimore at oracle.com]

I thought that this code under Verbose should have been changed to use 
logging with log_level = verbose.  We were trying to minimize/remove the 
Verbose flag.

On 5/14/20 9:50 AM, David Holmes wrote:
> +1
>
> But I'd be inclined to generalize the test to run:
>
> -Xlog:all=trace -XX:+Verbose

Don't do this, it would be tons of output and make the test unusable if 
it fails.

Coleen

>
> Curiously I couldn't reproduce the crash that this bug is about. But 
> the hs_err file speaks for itself.
>
> Cheers,
> David
>
> On 14/05/2020 11:28 pm, Daniel D. Daugherty wrote:
>> On 5/14/20 4:00 AM, Christian Hagedorn wrote:
>>> Hi David, hi Daniel
>>>
>>> Thank you for your reviews!
>>>
>>> On 13.05.20 17:54, Daniel D. Daugherty wrote:
>>>>      Not your bug, but the comment has two typos:
>>>>
>>>>          s/by safer/but safer/
>>>>          s/unexpensive/inexpensive/
>>>
>>> Thanks, fixed it.
>>>
>>> On 14.05.20 01:51, David Holmes wrote:
>>>> src/hotspot/cpu/x86/methodHandles_x86.cpp
>>>>
>>>> Given all of the code is inside:
>>>>
>>>>   504   if (Verbose) {
>>>>
>>>> I would just move the ResourceMark to the top of that block. We are 
>>>> going to use it unconditionally, and we are not releasing it until 
>>>> the end of the method regardless. That avoids the potential concern 
>>>> Dan raised about extending the scope of the PRESERVE_EXCEPTION_MARK.
>>>
>>> This sounds reasonable. I moved it to the top of that block.
>>>
>>>>> There are other places like Klass::print_on(outputStream*) where 
>>>>> this is still the case. Should we file an RFE to clean these up?
>>>>
>>>> Obviously we don't have any tests running in a mode that causes 
>>>> this code to be executed, so yes it is probably a good idea to take 
>>>> a closer look at other call-sites and file a RFE if needed.
>>>
>>> Yes, I filed one to take a closer look at these [1].
>>>
>>> I included the changes in a new webrev:
>>> http://cr.openjdk.java.net/~chagedorn/8244946/webrev.01/
>>
>> Thumbs up.
>>
>> Dan
>>
>>>
>>> Best regards,
>>> Christian
>>>
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8244999
>>>
>>