Patch attached.
Ran another 1000 clean testruns with the sleep(1)
pauses. Restored the sleep(100) for the final patch.
On 9/26/18, 2:12 PM, Chris Plummer wrote:
On 9/26/18 11:07 AM, Gary Adams wrote:
On 9/26/18, 1:40 PM, Chris Plummer wrote:
Hi Gary,
Should the following comment come first, not after the join() call:
115 mt.join();
116 // Wait till the other thread completes its execution.
I'll move the comment up.
Rather than using JVMTI to detect if the field is suspended,
couldn't you have just set a static variable in
callbackFieldAccess() and check it from isSuspended()?
All of the other native calls take a thread and operate on it.
It seemed reasonable to use the same check that popThreadFrame used.
Before doing the fix, did you first check if the bug is easily
reproduced by making is sleep for 1ms instead of 100ms?
That's how I got the problem to reproduce.
Switching to sleep(1) got 5 failures in 300 testruns.
Ok, and I assume you then tested the fix with the 1ms sleep? If so,
then ship it. Otherwise do this testing first.
thanks,
Chris
thanks,
Chris
On 9/26/18 7:55 AM, Gary Adams wrote:
A race condition exists in hs203t003 between the main test thread and
the processing in callbackFieldAccess processing. The main thread
already includes a polling loop to wait for the redefine class
operation
to be performed, but it does not wait for the following suspend thread
operation to be completed.
This changeset adds an additional wait for the suspend thread to
complete.
It also checks the error returns from popThreadFrame and resumeThread
to issue an additional error message if these native routines
returned an error.
Webrev: http://cr.openjdk.java.net/~gadams/8210984/webrev.00/
Issue: https://bugs.openjdk.java.net/browse/JDK-8210984
# HG changeset patch
# User gadams
# Date 1538047993 14400
# Thu Sep 27 07:33:13 2018 -0400
# Node ID 2dc330dc1343a36ae38209da78f96d23c4e8f8d2
# Parent eb3e72f181af1945573507d98854479c748e34e5
8210984: [TESTBUG] hs203t003 fails with "# ERROR: hs203t003.cpp, 218:
NSK_CPP_STUB2 ( ResumeThread, jvmti, thread)"
Reviewed-by: cjplummer, jcbeyler
diff --git
a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.cpp
b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.cpp
---
a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.cpp
+++
b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.cpp
@@ -174,6 +174,23 @@
return JNI_OK;
}
+JNIEXPORT jboolean JNICALL
+Java_nsk_jvmti_scenarios_hotswap_HS203_hs203t003_hs203t003_isSuspended(JNIEnv
* jni,
+ jclass clas,
+ jthread thread) {
+ jboolean retvalue;
+ jint state;
+ retvalue = JNI_FALSE;
+ if ( ! NSK_JVMTI_VERIFY( NSK_CPP_STUB3(GetThreadState, jvmti, thread,
&state) ) ) {
+ nsk_printf(" Agent :: Error while getting thread state.\n");
+ nsk_jvmti_agentFailed();
+ } else {
+ if ( state & JVMTI_THREAD_STATE_SUSPENDED) {
+ retvalue = JNI_TRUE;
+ }
+ }
+ return retvalue;
+}
JNIEXPORT jboolean JNICALL
Java_nsk_jvmti_scenarios_hotswap_HS203_hs203t003_hs203t003_popThreadFrame(JNIEnv
* jni,
diff --git
a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.java
b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.java
---
a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.java
+++
b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.java
@@ -62,7 +62,8 @@
public class hs203t003 extends RedefineAgent {
public native boolean popThreadFrame(Thread thread);
- public native boolean resumeThread(Thread thread);
+ public native boolean isSuspended(Thread thread);
+ public native boolean resumeThread(Thread thread);
public hs203t003(String[] arg) {
@@ -82,10 +83,10 @@
MyThread mt = new MyThread();
try {
mt.start();
- // check if we can can pop the thread.
- // we can not do redefine/pop frame on run method.
+ // Check if we can can pop the thread.
+ // We can not do redefine/pop frame on run method.
while (!MyThread.resume.get());
- // sleep for some few secs to get redefined.
+ // Sleep for some few secs to get redefined.
while (!isRedefined()) {
if (!agentStatus()) {
System.out.println("Failed to redefine class");
@@ -93,10 +94,26 @@
}
Thread.sleep(100);
}
- popThreadFrame(mt); // pop the frame.
- resumeThread(mt); // resume the thread.
+ // Wait for the thread to be suspended.
+ while (!isSuspended(mt)) {
+ if (!agentStatus()) {
+ System.out.println("Failed to suspend thread");
+ return passed;
+ }
+ Thread.sleep(100);
+ }
+ // Pop the frame.
+ if (!popThreadFrame(mt)) {
+ System.out.println("Failed to pop a frame = "
+ + mt.threadState);
+ }
+ // Resume the thread.
+ if(!resumeThread(mt)) {
+ System.out.println("Failed to resume the thread = "
+ + mt.threadState);
+ }
+ // Wait till the other thread completes its execution.
mt.join();
- // wait till the other thread completes its execution.
System.out.println("Thread state after popping/redefining = "
+ mt.threadState);
} catch(Exception ie) {
