Skip to content

Commit ca2efb7

Browse files
committed
8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend
Reviewed-by: cjplummer, sspitsyn, rschmelter
1 parent 296780c commit ca2efb7

File tree

2 files changed

+319
-19
lines changed

2 files changed

+319
-19
lines changed

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

Lines changed: 74 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ typedef struct ThreadNode {
7373
unsigned int popFrameEvent : 1;
7474
unsigned int popFrameProceed : 1;
7575
unsigned int popFrameThread : 1;
76+
unsigned int handlingAppResume : 1;
7677
EventIndex current_ei; /* Used to determine if we are currently handling an event on this thread. */
7778
jobject pendingStop; /* Object we are throwing to stop the thread (ThreadReferenceImpl.stop). */
7879
jint suspendCount;
@@ -679,7 +680,10 @@ pendingAppResume(jboolean includeSuspended)
679680
if (error != JVMTI_ERROR_NONE) {
680681
EXIT_ERROR(error, "getting thread state");
681682
}
682-
if (!(state & JVMTI_THREAD_STATE_SUSPENDED)) {
683+
/* !node->handlingAppResume && resumeFrameDepth > 0
684+
* means the thread has entered Thread.resume() */
685+
if (!(state & JVMTI_THREAD_STATE_SUSPENDED) &&
686+
!node->handlingAppResume) {
683687
return JNI_TRUE;
684688
}
685689
}
@@ -752,6 +756,11 @@ blockOnDebuggerSuspend(jthread thread)
752756
}
753757
}
754758

759+
/*
760+
* The caller is expected to hold threadLock and handlerLock.
761+
* eventHandler_createInternalThreadOnly() can deadlock because of
762+
* wrong lock ordering if the caller does not hold handlerLock.
763+
*/
755764
static void
756765
trackAppResume(jthread thread)
757766
{
@@ -800,28 +809,19 @@ handleAppResumeBreakpoint(JNIEnv *env, EventInfo *evinfo,
800809
struct bag *eventBag)
801810
{
802811
jthread resumer = evinfo->thread;
803-
jthread resumee = getResumee(resumer);
804812

805813
debugMonitorEnter(threadLock);
806-
if (resumee != NULL) {
807-
/*
808-
* Hold up any attempt to resume as long as the debugger
809-
* has suspended the resumee.
810-
*/
811-
blockOnDebuggerSuspend(resumee);
812-
}
813814

815+
/*
816+
* Actual handling has to be deferred. We cannot block right here if the
817+
* target of the resume call is suspended by the debugger since we are
818+
* holding handlerLock which must not be released. See doPendingTasks().
819+
*/
814820
if (resumer != NULL) {
815-
/*
816-
* Track the resuming thread by marking it as being within
817-
* a resume and by setting up for notification on
818-
* a frame pop or exception. We won't allow the debugger
819-
* to suspend threads while any thread is within a
820-
* call to resume. This (along with the block above)
821-
* ensures that when the debugger
822-
* suspends a thread it will remain suspended.
823-
*/
824-
trackAppResume(resumer);
821+
ThreadNode* node = findThread(&runningThreads, resumer);
822+
if (node != NULL) {
823+
node->handlingAppResume = JNI_TRUE;
824+
}
825825
}
826826

827827
debugMonitorExit(threadLock);
@@ -2179,6 +2179,59 @@ threadControl_onEventHandlerEntry(jbyte sessionID, EventInfo *evinfo, jobject cu
21792179
static void
21802180
doPendingTasks(JNIEnv *env, ThreadNode *node)
21812181
{
2182+
/* Deferred breakpoint handling for Thread.resume() */
2183+
if (node->handlingAppResume) {
2184+
jthread resumer = node->thread;
2185+
jthread resumee = getResumee(resumer);
2186+
2187+
if (resumer != NULL) {
2188+
/*
2189+
* trackAppResume indirectly aquires handlerLock. For proper lock
2190+
* ordering handlerLock has to be acquired before threadLock.
2191+
*/
2192+
debugMonitorExit(threadLock);
2193+
eventHandler_lock();
2194+
debugMonitorEnter(threadLock);
2195+
2196+
/*
2197+
* Track the resuming thread by marking it as being within
2198+
* a resume and by setting up for notification on
2199+
* a frame pop or exception. We won't allow the debugger
2200+
* to suspend threads while any thread is within a
2201+
* call to resume. This (along with the block below)
2202+
* ensures that when the debugger
2203+
* suspends a thread it will remain suspended.
2204+
*/
2205+
trackAppResume(resumer);
2206+
2207+
/*
2208+
* handlerLock is not needed anymore. We must release it before calling
2209+
* blockOnDebuggerSuspend() because it is required for resumes done by
2210+
* the debugger. If resumee is currently suspended by the debugger, then
2211+
* blockOnDebuggerSuspend() will block until a debugger resume is done.
2212+
* If it blocks while holding the handlerLock, then the resume will deadlock.
2213+
*/
2214+
eventHandler_unlock();
2215+
}
2216+
2217+
if (resumee != NULL) {
2218+
/*
2219+
* Hold up any attempt to resume as long as the debugger
2220+
* has suspended the resumee.
2221+
*/
2222+
blockOnDebuggerSuspend(resumee);
2223+
}
2224+
2225+
node->handlingAppResume = JNI_FALSE;
2226+
2227+
/*
2228+
* The blocks exit condition: resumee's suspendCount == 0.
2229+
*
2230+
* Debugger suspends are blocked if any thread is executing
2231+
* Thread.resume(), i.e. !handlingAppResume && frameDepth > 0.
2232+
*/
2233+
}
2234+
21822235
/*
21832236
* Take care of any pending interrupts/stops, and clear out
21842237
* info on pending interrupts/stops.
@@ -2479,6 +2532,8 @@ threadControl_reset(void)
24792532
/* Everything should have been resumed */
24802533
JDI_ASSERT(otherThreads.first == NULL);
24812534

2535+
/* Threads could be waiting in blockOnDebuggerSuspend */
2536+
debugMonitorNotifyAll(threadLock);
24822537
debugMonitorExit(threadLock);
24832538
eventHandler_unlock();
24842539
}
Lines changed: 245 additions & 0 deletions
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)