RFR: JDK-8216528: test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java failing with Xcomp

[Jie Fu]

Thanks David and Roger.


On 2019年01月12日 06:52, David Holmes wrote:
> Hi Roger,
>
> On 12/01/2019 2:22 am, Roger Riggs wrote:
>> Hi,
>>
>> The proposed patch changes the test in a way that is unintended.
>>
>> Adding the infinite loop of gc() and sleep, will change the timeout 
>> behavior
>> from the existing timeout of TIMEOUT to the jtreg default timeout of 
>> the whole test.
>
> Partially true. If the new loop gets stuck then yes the jtreg default 
> timeout will apply - I don't see that is necessarily a bad thing. The 
> existing timeout only applies to the refQueue.remove operation itself, 
> you don't know how much time was spent before you got there, nor how 
> much will be spent after in the dumpThreads() calls - so the jtreg 
> timeout can still come into affect.
>
>> Further, it renders the check at lines 114-120 irrelevant since 
>> loaderRef.get()
>> will have returned null and the ref will have been enqueued by then.
>
> I wouldn't say irrelevant as it double-checks the interaction between 
> the ref.get() and the queue.remove() - the result of one should imply 
> the result of the other, but if enqueuing had a bug ....
>
>> While it is true that calling gc() only once is unreliable, a better 
>> fix is to
>> put the code from 108-120 in a loop with a fixed number of durations
>
> That would also work - say 5 loops and reduce TIMEOUT to 4000.
>
>> and add Reachability.reachabilityFence(loaderRef) to ensure the ref 
>> is not ignored.
>
> Adding ReachabilityFence, alone, may solve the observed problem given 
> one gc() seems to be working in practice (and because we don't 
> actually have the leaked loaders anymore because those threads 
> (sun.misc.GC threads) don't exist anymore).
>
> Cheers,
> David
>
>> Regards, Roger