RFR 8209844: MemberNameLeak.java fails when ResolvedMethod entry is not removed

[Patricio Chilano]

Hi David,

On 9/4/18 9:59 PM, David Holmes wrote:
> On 5/09/2018 10:11 AM, Patricio Chilano wrote:
>> On 9/3/18 12:27 AM, David Holmes wrote:
>>> I'm somewhat surprised this test didn't already fail intermittently 
>>> as it assumes a single explicit System.gc() is enough to trigger the 
>>> cleanup it is checking for! If that assumption is not valid then the 
>>> test will now hang (in a busy polling loop!) until the test harness 
>>> times it out and forcibly terminates it. (Previously it would have 
>>> failed due to the missing output immediately after the gc() call 
>>> returned.)
>> I see that System.gc() ends up calling 
>> Universe::heap()->collect(GCCause::_java_lang_system_gc) which I 
>> understand will always reach SystemDictionary::do_unloading() 
>> eventually before the call returns. That would guarantee the table 
>> will be cleaned up before the program exits (provided the busy loop 
>> is there).
> There are tests that verify GC related actions (weak reference 
> processing, reference queues etc) that have to call System.gc() more 
> than once to actually get the expected actions to happen. It's not 
> easy (for me anyway) to determine exactly which actions are guaranteed 
> with a single System.gc() and which may require more. At a minimum 
> we're relying on current implementation to assume a single System.gc() 
> suffices. There are also flags that can affect this - though this test 
> is not likely to ever be run with them.

By looking at the different GCs it seems for System.gc() to reach 
SystemDictionary::do_unloading() they need 
GCLocker::check_active_before_gc() to return false, which will be do if 
_jni_lock_count=0, but I'm not sure when that can be guaranteed.
Some tests are relying on using ClassUnloadCommon.triggerUnloading() 
with flag "-Xmn8m" to trigger class unloading. I can replace System.gc() 
with that to make the assumption stronger. Do you think that works better?

Thanks,
Patricio

>>> On 29/08/2018 7:52 AM, Patricio Chilano wrote:
>>>> Hi all,
>>>>
>>>> Could you review this change that addresses the intermittent 
>>>> failure of test MemberNameLeak.java after 8206423 ?
>>>> The test had intermittent failures due to the ServiceThread not 
>>>> being able to clean up the resolved method table before the process 
>>>> exited. Now a counter is used to guarantee entries in the table are 
>>>> removed before exiting. Also variables _oops_removed and 
>>>> _oops_counted were declared local in ResolvedMethodTable::unlink() 
>>>> since they are not used outside it.
>>>> Webrev URL: http://cr.openjdk.java.net/~pchilanomate/8209844.01/webrev
>>>> Bug URL: https://bugs.openjdk.java.net/browse/JDK-8209844
>>>>
>>>> Thanks,
>>>> Patricio
>>