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