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

[David Holmes]

+1

But I'd be inclined to generalize the test to run:

-Xlog:all=trace -XX:+Verbose

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