[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.larsen at oracle.com]
Sent: 27 January 2014 16:14
To: Chan, Sunny [Tech]
Cc: serviceability-dev at openjdk.java.net serviceability-dev at 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.Chan at gs.com<mailto:Sunny.Chan at 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.larsen at oracle.com]
Sent: 13 January 2014 12:37
To: Chan, Sunny [Tech]
Cc: serviceability-dev at openjdk.java.net<mailto:serviceability-dev at openjdk.java.net> serviceability-dev at openjdk.java.net<mailto:serviceability-dev at 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.Chan at gs.com<mailto:Sunny.Chan at gs.com>> wrote:



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

Yes, it does.

Thanks,
/Staffan
<7142035rev1.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140128/7a7b13ca/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 7142035rev2.patch
Type: application/octet-stream
Size: 10842 bytes
Desc: 7142035rev2.patch
Url : http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140128/7a7b13ca/7142035rev2-0001.patch