RE: [PATCH] 7142035 assert in j.l.instrument agents during shutdown when daemon thread is running

[Chan, Sunny]

Hi Staffan,

I agreed with your assessment and have incorporated your extra change into my 
patch. I have a couple of test runs and have determined that we can reduce each 
process period but we will need to repeat at least 50 times to reliably 
reproduce the issue in unpatched builds. So the intervals is now 200ms instead 
of 1 second.

Let me know if there is anything else need to change.

Thanks
Sunny

From: Staffan Larsen [mailto:staffan.lar...@oracle.com]
Sent: 27 January 2014 16:14
To: Chan, Sunny [Tech]
Cc: serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net
Subject: Re: [PATCH] 7142035 assert in j.l.instrument agents during shutdown 
when daemon thread is running

Hi Sunny,

This looks good!

- I'd like to avoid bug ids in the names of tests, so can you change the 
directory name to say "DaemonThread"?

- The test takes quite a long time to run (~1 minute). I wonder if we should 
shorten that by either starting less processes (currently 50) or letting each 
process run for a shorter period (currently ~1 sec).

- I think we can reduce the noise by removing the System.out.println() output 
from the DummyAgent.

- During one of my test runs I ran into the following failure:
*** java.lang.instrument ASSERTION FAILED ***: "error == JVMTI_ERROR_NONE" at 
Reentrancy.c line: 133
It looks like the patch isn't complete. I added the patch below and have not 
seen a failure after that. Can you incorporate that change as well (if you 
agree with it)?

Thanks,
/Staffan


--- a/src/share/instrument/Reentrancy.c
+++ b/src/share/instrument/Reentrancy.c
@@ -130,6 +130,7 @@
             error = confirmingTLSSet (  jvmtienv,
                                         thread,
                                         JPLIS_CURRENTLY_INSIDE_TOKEN);
+            check_phase_ret_false(error);
             jplis_assert(error == JVMTI_ERROR_NONE);
             if ( error != JVMTI_ERROR_NONE ) {
                 result = JNI_FALSE;


On 27 jan 2014, at 12:44, Chan, Sunny 
<sunny.c...@gs.com<mailto:sunny.c...@gs.com>> wrote:


Hi Staffan,

I have attached the new version of the patch - I have reworked the test case 
and now it is mostly based in Java, but I have decided to keep using the shell 
script to build the Agent Jar file as it is easier.

Thanks.

From: Staffan Larsen [mailto:staffan.lar...@oracle.com]
Sent: 13 January 2014 12:37
To: Chan, Sunny [Tech]
Cc: 
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net> 
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: Re: [PATCH] 7142035 assert in j.l.instrument agents during shutdown 
when daemon thread is running


On 8 jan 2014, at 16:48, Chan, Sunny 
<sunny.c...@gs.com<mailto:sunny.c...@gs.com>> wrote:



In terms of the bug fix itself does it look fine?

Yes, it does.

Thanks,
/Staffan
<7142035rev1.patch>

7142035rev2.patch
Description: 7142035rev2.patch