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

[Roger Riggs]

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.

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.

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
and add Reachability.reachabilityFence(loaderRef) to ensure the ref is 
not ignored.

Regards, Roger


On 01/11/2019 12:07 AM, Jie Fu wrote:
> Hi David,
>
> Thank you very much. I'd like to choose option 2.
> A test case is more valuable if it can be used for both interpreter 
> and JIT tests.
>
> Here is the patch based on your comments.
> ---------------------------------------------------------------------------------- 
>
> diff -r 02e648ae46c3 
> test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java
> --- 
> a/test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java 
> Wed Jan 09 01:06:19 2019 +0100
> +++ 
> b/test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java 
> Fri Jan 11 12:55:38 2019 +0800
> @@ -22,7 +22,7 @@
>   */
>
>  /* @test
> - * @bug 4404702
> + * @bug 4404702 8216528
>   * @summary When the RMI runtime (lazily) spawns system threads that 
> could
>   * outlive the application context in which they were (happened to be)
>   * created, such threads should not inherit (thread local) data 
> specific to
> @@ -106,7 +106,10 @@
>               * context class loader-- by giving it a chance to pass 
> away.
>               */
>              Thread.sleep(2000);
> -            System.gc();
> +            while (loaderRef.get() != null) {
> +                System.gc();
> +                Thread.sleep(100);
> +            }
>
>              System.err.println(
>                  "waiting to be notified of loader being weakly 
> reachable...");
> ---------------------------------------------------------------------------------- 
>
>
> Could you please review it and give me some advice?
> Thanks.
>
> Best regards,
> Jie
>
>
> On 2019/1/11 下午12:16, David Holmes wrote:
>>
>> I see three choices for you here :)
>>
>> 1. Don't try to run all tests under Xcomp but just stick to the 
>> "core" sets of tests already tested by others.
>>
>> 2. Fix the given test as outlined. (I tested it on linux-x64 and it 
>> fixed the problem.)
>>
>> 3. Exclude the given test from Xcomp by adding: @requires vm.compMode 
>> != "Xcomp"
>>
>> If you chose options 2 or 3 please update the @bug line with 8216528.
>>
>> The core-libs folk may have more to say here and they will need to 
>> provide a sponsor for the commit.
>>
>> Thanks,
>> David
>> -----
>