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