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

[David Holmes]

On 15/05/2020 12:02 am, coleen.phillimore at oracle.com wrote:
> 
> 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.

The only way the test can fail is if it crashes.

But we would want a level of indirection so that the test log doesn't 
contain the full output ~11000 lines of text.

David
-----

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