Skip to content

Commit e568880

Browse files
committed
8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend
Reviewed-by: mdoerr Backport-of: ca2efb73f59112d9be2ec29db405deb4c58dd435
1 parent 9e8917b commit e568880

File tree

3 files changed

+326
-20
lines changed

3 files changed

+326
-20
lines changed

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c

+74-19
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ typedef struct ThreadNode {
7171
unsigned int popFrameEvent : 1;
7272
unsigned int popFrameProceed : 1;
7373
unsigned int popFrameThread : 1;
74+
unsigned int handlingAppResume : 1;
7475
EventIndex current_ei;
7576
jobject pendingStop;
7677
jint suspendCount;
@@ -622,7 +623,10 @@ pendingAppResume(jboolean includeSuspended)
622623
if (error != JVMTI_ERROR_NONE) {
623624
EXIT_ERROR(error, "getting thread state");
624625
}
625-
if (!(state & JVMTI_THREAD_STATE_SUSPENDED)) {
626+
/* !node->handlingAppResume && resumeFrameDepth > 0
627+
* means the thread has entered Thread.resume() */
628+
if (!(state & JVMTI_THREAD_STATE_SUSPENDED) &&
629+
!node->handlingAppResume) {
626630
return JNI_TRUE;
627631
}
628632
}
@@ -691,6 +695,11 @@ blockOnDebuggerSuspend(jthread thread)
691695
}
692696
}
693697

698+
/*
699+
* The caller is expected to hold threadLock and handlerLock.
700+
* eventHandler_createInternalThreadOnly() can deadlock because of
701+
* wrong lock ordering if the caller does not hold handlerLock.
702+
*/
694703
static void
695704
trackAppResume(jthread thread)
696705
{
@@ -738,28 +747,19 @@ handleAppResumeBreakpoint(JNIEnv *env, EventInfo *evinfo,
738747
struct bag *eventBag)
739748
{
740749
jthread resumer = evinfo->thread;
741-
jthread resumee = getResumee(resumer);
742750

743751
debugMonitorEnter(threadLock);
744-
if (resumee != NULL) {
745-
/*
746-
* Hold up any attempt to resume as long as the debugger
747-
* has suspended the resumee.
748-
*/
749-
blockOnDebuggerSuspend(resumee);
750-
}
751752

753+
/*
754+
* Actual handling has to be deferred. We cannot block right here if the
755+
* target of the resume call is suspended by the debugger since we are
756+
* holding handlerLock which must not be released. See doPendingTasks().
757+
*/
752758
if (resumer != NULL) {
753-
/*
754-
* Track the resuming thread by marking it as being within
755-
* a resume and by setting up for notification on
756-
* a frame pop or exception. We won't allow the debugger
757-
* to suspend threads while any thread is within a
758-
* call to resume. This (along with the block above)
759-
* ensures that when the debugger
760-
* suspends a thread it will remain suspended.
761-
*/
762-
trackAppResume(resumer);
759+
ThreadNode* node = findThread(&runningThreads, resumer);
760+
if (node != NULL) {
761+
node->handlingAppResume = JNI_TRUE;
762+
}
763763
}
764764

765765
debugMonitorExit(threadLock);
@@ -2107,6 +2107,59 @@ threadControl_onEventHandlerEntry(jbyte sessionID, EventIndex ei, jthread thread
21072107
static void
21082108
doPendingTasks(JNIEnv *env, ThreadNode *node)
21092109
{
2110+
/* Deferred breakpoint handling for Thread.resume() */
2111+
if (node->handlingAppResume) {
2112+
jthread resumer = node->thread;
2113+
jthread resumee = getResumee(resumer);
2114+
2115+
if (resumer != NULL) {
2116+
/*
2117+
* trackAppResume indirectly aquires handlerLock. For proper lock
2118+
* ordering handlerLock has to be acquired before threadLock.
2119+
*/
2120+
debugMonitorExit(threadLock);
2121+
eventHandler_lock();
2122+
debugMonitorEnter(threadLock);
2123+
2124+
/*
2125+
* Track the resuming thread by marking it as being within
2126+
* a resume and by setting up for notification on
2127+
* a frame pop or exception. We won't allow the debugger
2128+
* to suspend threads while any thread is within a
2129+
* call to resume. This (along with the block below)
2130+
* ensures that when the debugger
2131+
* suspends a thread it will remain suspended.
2132+
*/
2133+
trackAppResume(resumer);
2134+
2135+
/*
2136+
* handlerLock is not needed anymore. We must release it before calling
2137+
* blockOnDebuggerSuspend() because it is required for resumes done by
2138+
* the debugger. If resumee is currently suspended by the debugger, then
2139+
* blockOnDebuggerSuspend() will block until a debugger resume is done.
2140+
* If it blocks while holding the handlerLock, then the resume will deadlock.
2141+
*/
2142+
eventHandler_unlock();
2143+
}
2144+
2145+
if (resumee != NULL) {
2146+
/*
2147+
* Hold up any attempt to resume as long as the debugger
2148+
* has suspended the resumee.
2149+
*/
2150+
blockOnDebuggerSuspend(resumee);
2151+
}
2152+
2153+
node->handlingAppResume = JNI_FALSE;
2154+
2155+
/*
2156+
* The blocks exit condition: resumee's suspendCount == 0.
2157+
*
2158+
* Debugger suspends are blocked if any thread is executing
2159+
* Thread.resume(), i.e. !handlingAppResume && frameDepth > 0.
2160+
*/
2161+
}
2162+
21102163
/*
21112164
* Take care of any pending interrupts/stops, and clear out
21122165
* info on pending interrupts/stops.
@@ -2407,6 +2460,8 @@ threadControl_reset(void)
24072460
/* Everything should have been resumed */
24082461
JDI_ASSERT(otherThreads.first == NULL);
24092462

2463+
/* Threads could be waiting in blockOnDebuggerSuspend */
2464+
debugMonitorNotifyAll(threadLock);
24102465
debugMonitorExit(threadLock);
24112466
eventHandler_unlock();
24122467
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
/*
2+
* Copyright (c) 2021 SAP SE. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8274687
27+
* @summary Test the special handling in the JDWP agent of threads that call
28+
* j.l.Thread.resume().
29+
*
30+
* This is the sequence of actions by the debugger and the threads
31+
* "main" and "resumee" in the target vm.
32+
*
33+
* "resumee": Reaches breakpoint in methodWithBreakpoint() and is
34+
* suspended then.
35+
*
36+
* "main": Calls j.l.Thread.resume() for "resumee". There is an internal
37+
* breakpoint in Thread.resume() so the JDWP agent receives a
38+
* breakpoint event. It finds that "resumee" is suspended because
39+
* of JDWP actions. The resume() call would interfere with the
40+
* debugger therefore "main" is blocked.
41+
*
42+
* Debugger: Tests if the suspended "resumee" can be suspended a second
43+
* time and resumes it again.
44+
*
45+
* Debugger: Resumes "resumee" by calling ThreadReference.resume().
46+
* The JDWP agent notifies "main" about it.
47+
*
48+
* "resumee": Continues execution.
49+
*
50+
* "main": Receives the notification, finds that "resumee" is not
51+
* suspended anymore and continues execution.
52+
*
53+
* Debugger: Verifies that "main" is no longer blocked.
54+
*
55+
* @author Richard Reingruber richard DOT reingruber AT sap DOT com
56+
*
57+
* @library /test/lib
58+
*
59+
* @run build TestScaffold VMConnection TargetListener TargetAdapter
60+
* @run compile -g ResumeAfterThreadResumeCallTest.java
61+
* @run driver ResumeAfterThreadResumeCallTest
62+
*/
63+
import com.sun.jdi.*;
64+
import com.sun.jdi.event.*;
65+
import jdk.test.lib.Asserts;
66+
67+
import java.util.*;
68+
69+
// Target program for the debugger
70+
class ResumeAfterThreadResumeCallTarg extends Thread {
71+
72+
public boolean reachedBreakpoint;
73+
public boolean mainThreadReturnedFromResumeCall;
74+
public boolean testFinished;
75+
76+
public ResumeAfterThreadResumeCallTarg(String name) {
77+
super(name);
78+
}
79+
80+
public static void log(String m) {
81+
String threadName = Thread.currentThread().getName();
82+
System.out.println("###(Target,"+ threadName +") " + m);
83+
}
84+
85+
public static void main(String[] args) {
86+
log("Entered main()");
87+
88+
// Start "resumee" thread.
89+
ResumeAfterThreadResumeCallTarg resumee = new ResumeAfterThreadResumeCallTarg("resumee");
90+
resumee.start();
91+
92+
// Wait for "resumee" to reach the breakpoint in methodWithBreakpoint().
93+
while (!resumee.reachedBreakpoint) {
94+
try {
95+
Thread.sleep(100);
96+
} catch (InterruptedException e) { /* ignored */ }
97+
}
98+
99+
// "resumee" is suspended now because of the breakpoint
100+
// Calling Thread.resume() will block this thread.
101+
log("Calling Thread.resume()");
102+
resumee.resume();
103+
resumee.mainThreadReturnedFromResumeCall = true;
104+
log("Thread.resume() returned");
105+
106+
// Wait for debugger
107+
while (!resumee.testFinished) {
108+
try {
109+
Thread.sleep(100);
110+
} catch (InterruptedException e) { /* ignored */ }
111+
}
112+
}
113+
114+
public void run() {
115+
log("up and running.");
116+
methodWithBreakpoint();
117+
}
118+
119+
public void methodWithBreakpoint() {
120+
log("Entered methodWithBreakpoint()");
121+
}
122+
}
123+
124+
125+
// Debugger program
126+
127+
public class ResumeAfterThreadResumeCallTest extends TestScaffold {
128+
public static final String TARGET_CLS_NAME = ResumeAfterThreadResumeCallTarg.class.getName();
129+
public static final long UNBLOCK_TIMEOUT = 10000;
130+
131+
ResumeAfterThreadResumeCallTest (String args[]) {
132+
super(args);
133+
}
134+
135+
public static void main(String[] args) throws Exception {
136+
new ResumeAfterThreadResumeCallTest(args).startTests();
137+
}
138+
139+
/**
140+
* Set a breakpoint in the given method and resume all threads. The
141+
* breakpoint is configured to suspend just the thread that reaches it
142+
* instead of all threads.
143+
*/
144+
public BreakpointEvent resumeTo(String clsName, String methodName, String signature) {
145+
boolean suspendThreadOnly = true;
146+
return resumeTo(clsName, methodName, signature, suspendThreadOnly);
147+
}
148+
149+
protected void runTests() throws Exception {
150+
BreakpointEvent bpe = startToMain(TARGET_CLS_NAME);
151+
mainThread = bpe.thread();
152+
153+
log("Resuming to methodWithBreakpoint()");
154+
bpe = resumeTo(TARGET_CLS_NAME, "methodWithBreakpoint", "()V");
155+
156+
log("Thread \"resumee\" has reached the breakpoint and is suspended now.");
157+
ThreadReference resumee = bpe.thread();
158+
ObjectReference resumeeThreadObj = resumee.frame(1).thisObject();
159+
printStack(resumee);
160+
mainThread.suspend();
161+
printStack(mainThread);
162+
mainThread.resume();
163+
log("resumee.isSuspended() -> " + resumee.isSuspended());
164+
log("mainThread.isSuspended() -> " + mainThread.isSuspended());
165+
log("Notify target main thread to continue by setting reachedBreakpoint = true.");
166+
setField(resumeeThreadObj, "reachedBreakpoint", vm().mirrorOf(true));
167+
168+
log("Sleeping 500ms so that the main thread is blocked calling Thread.resume() on \"resumee\" Thread.");
169+
Thread.sleep(500);
170+
log("After sleep.");
171+
mainThread.suspend();
172+
printStack(mainThread);
173+
mainThread.resume();
174+
175+
boolean mainThreadReturnedFromResumeCall = false;
176+
boolean resumedResumee = false;
177+
for (long sleepTime = 50; sleepTime < UNBLOCK_TIMEOUT && !mainThreadReturnedFromResumeCall; sleepTime <<= 1) {
178+
log("mainThread.isSuspended() -> " + mainThread.isSuspended());
179+
Value v = getField(resumeeThreadObj, "mainThreadReturnedFromResumeCall");
180+
mainThreadReturnedFromResumeCall = ((PrimitiveValue) v).booleanValue();
181+
if (!resumedResumee) {
182+
// main thread should still be blocked.
183+
Asserts.assertFalse(mainThreadReturnedFromResumeCall, "main Thread was not blocked");
184+
185+
// Test suspending the already suspended resumee thread.
186+
Asserts.assertTrue(resumee.isSuspended(), "\"resumee\" is not suspended.");
187+
log("Check if suspended \"resumee\" can be suspended a 2nd time.");
188+
resumee.suspend();
189+
log("resumee.isSuspended() -> " + resumee.isSuspended());
190+
log("Resuming \"resumee\"");
191+
resumee.resume();
192+
Asserts.assertTrue(resumee.isSuspended(), "\"resumee\" is not suspended.");
193+
194+
// Really resume the resumee thread.
195+
log("Resuming \"resumee\" a 2nd time will unblock the main thread.");
196+
resumee.resume();
197+
Asserts.assertFalse(resumee.isSuspended(), "\"resumee\" is still suspended.");
198+
resumedResumee = true;
199+
}
200+
log("Sleeping " + sleepTime + "ms");
201+
Thread.sleep(sleepTime);
202+
}
203+
Asserts.assertTrue(mainThreadReturnedFromResumeCall, "main Thread was not unblocked");
204+
205+
setField(resumeeThreadObj, "testFinished", vm().mirrorOf(true));
206+
207+
// Resume the target listening for events
208+
listenUntilVMDisconnect();
209+
}
210+
211+
public void printStack(ThreadReference thread) throws Exception {
212+
log("Stack of thread '" + thread.name() + "':");
213+
List<StackFrame> stack_frames = thread.frames();
214+
int i = 0;
215+
for (StackFrame ff : stack_frames) {
216+
Location loc = ff.location();
217+
String locString = "bci:" + loc.codeIndex();
218+
try {
219+
locString = loc.sourceName() + ":" + loc.lineNumber() + "," + locString;
220+
} catch (AbsentInformationException e) {/* empty */};
221+
log(" frame[" + i++ +"]: " + ff.location().method() + " (" + locString + ")");
222+
}
223+
}
224+
225+
public void setField(ObjectReference obj, String fName, Value val) throws Exception {
226+
log("set field " + fName + " = " + val);
227+
ReferenceType rt = obj.referenceType();
228+
Field fld = rt.fieldByName(fName);
229+
obj.setValue(fld, val);
230+
log("ok");
231+
}
232+
233+
public Value getField(ObjectReference obj, String fName) throws Exception {
234+
log("get field " + fName);
235+
ReferenceType rt = obj.referenceType();
236+
Field fld = rt.fieldByName(fName);
237+
Value val = obj.getValue(fld);
238+
log("result : " + val);
239+
return val;
240+
}
241+
242+
public void log(String m) {
243+
System.out.println("###(Debugger) " + m);
244+
}
245+
}

0 commit comments

Comments
 (0)