Skip to content

Commit 230cd5e

Browse files
committed
8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend
Backport-of: ca2efb73f59112d9be2ec29db405deb4c58dd435
1 parent 296dc78 commit 230cd5e

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;
@@ -655,7 +656,10 @@ pendingAppResume(jboolean includeSuspended)
655656
if (error != JVMTI_ERROR_NONE) {
656657
EXIT_ERROR(error, "getting thread state");
657658
}
658-
if (!(state & JVMTI_THREAD_STATE_SUSPENDED)) {
659+
/* !node->handlingAppResume && resumeFrameDepth > 0
660+
* means the thread has entered Thread.resume() */
661+
if (!(state & JVMTI_THREAD_STATE_SUSPENDED) &&
662+
!node->handlingAppResume) {
659663
return JNI_TRUE;
660664
}
661665
}
@@ -728,6 +732,11 @@ blockOnDebuggerSuspend(jthread thread)
728732
}
729733
}
730734

735+
/*
736+
* The caller is expected to hold threadLock and handlerLock.
737+
* eventHandler_createInternalThreadOnly() can deadlock because of
738+
* wrong lock ordering if the caller does not hold handlerLock.
739+
*/
731740
static void
732741
trackAppResume(jthread thread)
733742
{
@@ -776,28 +785,19 @@ handleAppResumeBreakpoint(JNIEnv *env, EventInfo *evinfo,
776785
struct bag *eventBag)
777786
{
778787
jthread resumer = evinfo->thread;
779-
jthread resumee = getResumee(resumer);
780788

781789
debugMonitorEnter(threadLock);
782-
if (resumee != NULL) {
783-
/*
784-
* Hold up any attempt to resume as long as the debugger
785-
* has suspended the resumee.
786-
*/
787-
blockOnDebuggerSuspend(resumee);
788-
}
789790

791+
/*
792+
* Actual handling has to be deferred. We cannot block right here if the
793+
* target of the resume call is suspended by the debugger since we are
794+
* holding handlerLock which must not be released. See doPendingTasks().
795+
*/
790796
if (resumer != NULL) {
791-
/*
792-
* Track the resuming thread by marking it as being within
793-
* a resume and by setting up for notification on
794-
* a frame pop or exception. We won't allow the debugger
795-
* to suspend threads while any thread is within a
796-
* call to resume. This (along with the block above)
797-
* ensures that when the debugger
798-
* suspends a thread it will remain suspended.
799-
*/
800-
trackAppResume(resumer);
797+
ThreadNode* node = findThread(&runningThreads, resumer);
798+
if (node != NULL) {
799+
node->handlingAppResume = JNI_TRUE;
800+
}
801801
}
802802

803803
debugMonitorExit(threadLock);
@@ -2155,6 +2155,59 @@ threadControl_onEventHandlerEntry(jbyte sessionID, EventInfo *evinfo, jobject cu
21552155
static void
21562156
doPendingTasks(JNIEnv *env, ThreadNode *node)
21572157
{
2158+
/* Deferred breakpoint handling for Thread.resume() */
2159+
if (node->handlingAppResume) {
2160+
jthread resumer = node->thread;
2161+
jthread resumee = getResumee(resumer);
2162+
2163+
if (resumer != NULL) {
2164+
/*
2165+
* trackAppResume indirectly aquires handlerLock. For proper lock
2166+
* ordering handlerLock has to be acquired before threadLock.
2167+
*/
2168+
debugMonitorExit(threadLock);
2169+
eventHandler_lock();
2170+
debugMonitorEnter(threadLock);
2171+
2172+
/*
2173+
* Track the resuming thread by marking it as being within
2174+
* a resume and by setting up for notification on
2175+
* a frame pop or exception. We won't allow the debugger
2176+
* to suspend threads while any thread is within a
2177+
* call to resume. This (along with the block below)
2178+
* ensures that when the debugger
2179+
* suspends a thread it will remain suspended.
2180+
*/
2181+
trackAppResume(resumer);
2182+
2183+
/*
2184+
* handlerLock is not needed anymore. We must release it before calling
2185+
* blockOnDebuggerSuspend() because it is required for resumes done by
2186+
* the debugger. If resumee is currently suspended by the debugger, then
2187+
* blockOnDebuggerSuspend() will block until a debugger resume is done.
2188+
* If it blocks while holding the handlerLock, then the resume will deadlock.
2189+
*/
2190+
eventHandler_unlock();
2191+
}
2192+
2193+
if (resumee != NULL) {
2194+
/*
2195+
* Hold up any attempt to resume as long as the debugger
2196+
* has suspended the resumee.
2197+
*/
2198+
blockOnDebuggerSuspend(resumee);
2199+
}
2200+
2201+
node->handlingAppResume = JNI_FALSE;
2202+
2203+
/*
2204+
* The blocks exit condition: resumee's suspendCount == 0.
2205+
*
2206+
* Debugger suspends are blocked if any thread is executing
2207+
* Thread.resume(), i.e. !handlingAppResume && frameDepth > 0.
2208+
*/
2209+
}
2210+
21582211
/*
21592212
* Take care of any pending interrupts/stops, and clear out
21602213
* info on pending interrupts/stops.
@@ -2455,6 +2508,8 @@ threadControl_reset(void)
24552508
/* Everything should have been resumed */
24562509
JDI_ASSERT(otherThreads.first == NULL);
24572510

2511+
/* Threads could be waiting in blockOnDebuggerSuspend */
2512+
debugMonitorNotifyAll(threadLock);
24582513
debugMonitorExit(threadLock);
24592514
eventHandler_unlock();
24602515
}
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)