Skip to content

Commit f019292

Browse files
testforstephentestforstephen
committed
Fix race competition when the breakpoint needs be evaluated in multi threads
Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
1 parent 32f3a1f commit f019292

File tree

6 files changed

+119
-25
lines changed

6 files changed

+119
-25
lines changed

‎com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/EvaluatableBreakpoint.java‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,39 @@
1313

1414
import org.apache.commons.lang3.StringUtils;
1515

16+
import java.util.HashMap;
17+
import java.util.Map;
18+
import java.util.concurrent.CompletableFuture;
19+
20+
import com.sun.jdi.event.ThreadDeathEvent;
21+
import com.sun.jdi.ThreadReference;
1622
import com.sun.jdi.VirtualMachine;
1723

24+
import io.reactivex.disposables.Disposable;
25+
1826
public class EvaluatableBreakpoint extends Breakpoint implements IEvaluatableBreakpoint {
27+
private IEventHub eventHub = null;
1928
private Object compiledConditionalExpression = null;
2029
private Object compiledLogpointExpression = null;
30+
private Map<Long, Object> compiledExpressions = new HashMap<>();
2131

2232
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber) {
23-
super(vm, eventHub, className, lineNumber, 0, null);
33+
this(vm, eventHub, className, lineNumber, 0, null);
2434
}
2535

2636
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount) {
27-
super(vm, eventHub, className, lineNumber, hitCount, null);
37+
this(vm, eventHub, className, lineNumber, hitCount, null);
2838
}
2939

30-
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount, String condition) {
31-
super(vm, eventHub, className, lineNumber, hitCount, condition);
40+
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount,
41+
String condition) {
42+
this(vm, eventHub, className, lineNumber, hitCount, condition, null);
3243
}
3344

34-
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount, String condition, String logMessage) {
45+
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount,
46+
String condition, String logMessage) {
3547
super(vm, eventHub, className, lineNumber, hitCount, condition, logMessage);
48+
this.eventHub = eventHub;
3649
}
3750

3851
@Override
@@ -74,11 +87,36 @@ public Object getCompiledLogpointExpression() {
7487
public void setCondition(String condition) {
7588
super.setCondition(condition);
7689
setCompiledConditionalExpression(null);
90+
compiledExpressions.clear();
7791
}
7892

7993
@Override
8094
public void setLogMessage(String logMessage) {
8195
super.setLogMessage(logMessage);
8296
setCompiledLogpointExpression(null);
97+
compiledExpressions.clear();
98+
}
99+
100+
@Override
101+
public Object getCompiledExpression(long threadId) {
102+
return compiledExpressions.get(threadId);
103+
}
104+
105+
@Override
106+
public void setCompiledExpression(long threadId, Object compiledExpression) {
107+
compiledExpressions.put(threadId, compiledExpression);
108+
}
109+
110+
@Override
111+
public CompletableFuture<IBreakpoint> install() {
112+
Disposable subscription = eventHub.events()
113+
.filter(debugEvent -> debugEvent.event instanceof ThreadDeathEvent)
114+
.subscribe(debugEvent -> {
115+
ThreadReference deathThread = ((ThreadDeathEvent) debugEvent.event).thread();
116+
compiledExpressions.remove(deathThread.uniqueID());
117+
});
118+
super.subscriptions().add(subscription);
119+
120+
return super.install();
83121
}
84122
}

‎com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/IEvaluatableBreakpoint.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,46 @@ public interface IEvaluatableBreakpoint {
2626

2727
void setLogMessage(String logMessage);
2828

29+
/**
30+
* @deprecated please use {@link #setCompiledExpression(long, Object)} instead.
31+
*/
32+
@Deprecated
2933
void setCompiledConditionalExpression(Object compiledExpression);
3034

35+
/**
36+
* @deprecated please use {@link #getCompiledExpression(long)} instead.
37+
* @return
38+
*/
39+
@Deprecated
3140
Object getCompiledConditionalExpression();
3241

42+
/**
43+
* @deprecated please use {@link #setCompiledExpression(long, Object)} instead.
44+
*/
45+
@Deprecated
3346
void setCompiledLogpointExpression(Object compiledExpression);
3447

48+
/**
49+
* @deprecated please use {@link #getCompiledExpression(long)} instead.
50+
* @return
51+
*/
52+
@Deprecated
3553
Object getCompiledLogpointExpression();
54+
55+
/**
56+
* Sets the compiled expression for a thread.
57+
*
58+
* @param threadId - thread the breakpoint is hit in
59+
* @param compiledExpression - associated compiled expression
60+
*/
61+
void setCompiledExpression(long threadId, Object compiledExpression);
62+
63+
/**
64+
* Returns existing compiled expression for the given thread or
65+
* <code>null</code>.
66+
*
67+
* @param threadId thread the breakpoint was hit in
68+
* @return compiled expression or <code>null</code>
69+
*/
70+
Object getCompiledExpression(long threadId);
3671
}

‎com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Watchpoint.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,19 @@
1414
import java.util.ArrayList;
1515
import java.util.HashMap;
1616
import java.util.List;
17+
import java.util.Map;
1718
import java.util.Objects;
1819
import java.util.concurrent.CompletableFuture;
1920

2021
import org.apache.commons.lang3.StringUtils;
2122

2223
import com.sun.jdi.Field;
2324
import com.sun.jdi.ReferenceType;
25+
import com.sun.jdi.ThreadReference;
2426
import com.sun.jdi.VMDisconnectedException;
2527
import com.sun.jdi.VirtualMachine;
2628
import com.sun.jdi.event.ClassPrepareEvent;
29+
import com.sun.jdi.event.ThreadDeathEvent;
2730
import com.sun.jdi.request.ClassPrepareRequest;
2831
import com.sun.jdi.request.EventRequest;
2932
import com.sun.jdi.request.WatchpointRequest;
@@ -41,6 +44,7 @@ public class Watchpoint implements IWatchpoint, IEvaluatableBreakpoint {
4144
private int hitCount;
4245
private HashMap<Object, Object> propertyMap = new HashMap<>();
4346
private Object compiledConditionalExpression = null;
47+
private Map<Long, Object> compiledExpressions = new HashMap<>();
4448

4549
// IDebugResource
4650
private List<EventRequest> requests = new ArrayList<>();
@@ -116,6 +120,7 @@ public String getCondition() {
116120
public void setCondition(String condition) {
117121
this.condition = condition;
118122
setCompiledConditionalExpression(null);
123+
compiledExpressions.clear();
119124
}
120125

121126
@Override
@@ -147,6 +152,14 @@ public Object getProperty(Object key) {
147152

148153
@Override
149154
public CompletableFuture<IWatchpoint> install() {
155+
Disposable subscription = eventHub.events()
156+
.filter(debugEvent -> debugEvent.event instanceof ThreadDeathEvent)
157+
.subscribe(debugEvent -> {
158+
ThreadReference deathThread = ((ThreadDeathEvent) debugEvent.event).thread();
159+
compiledExpressions.remove(deathThread.uniqueID());
160+
});
161+
subscriptions.add(subscription);
162+
150163
// It's possible that different class loaders create new class with the same name.
151164
// Here to listen to future class prepare events to handle such case.
152165
ClassPrepareRequest classPrepareRequest = vm.eventRequestManager().createClassPrepareRequest();
@@ -155,7 +168,7 @@ public CompletableFuture<IWatchpoint> install() {
155168
requests.add(classPrepareRequest);
156169

157170
CompletableFuture<IWatchpoint> future = new CompletableFuture<>();
158-
Disposable subscription = eventHub.events()
171+
subscription = eventHub.events()
159172
.filter(debugEvent -> debugEvent.event instanceof ClassPrepareEvent && (classPrepareRequest.equals(debugEvent.event.request())))
160173
.subscribe(debugEvent -> {
161174
ClassPrepareEvent event = (ClassPrepareEvent) debugEvent.event;
@@ -249,4 +262,14 @@ public void setCompiledLogpointExpression(Object compiledExpression) {
249262
public Object getCompiledLogpointExpression() {
250263
return null;
251264
}
265+
266+
@Override
267+
public Object getCompiledExpression(long threadId) {
268+
return compiledExpressions.get(threadId);
269+
}
270+
271+
@Override
272+
public void setCompiledExpression(long threadId, Object compiledExpression) {
273+
compiledExpressions.put(threadId, compiledExpression);
274+
}
252275
}

‎com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/AbstractDisconnectRequestHandler.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public List<Command> getTargetCommands() {
4040
@Override
4141
public CompletableFuture<Response> handle(Command command, Arguments arguments, Response response,
4242
IDebugAdapterContext context) {
43+
context.setVmTerminated();
4344
destroyDebugSession(command, arguments, response, context);
4445
destroyResource(context);
4546
return CompletableFuture.completedFuture(response);

‎com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/SetBreakpointsRequestHandler.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.sun.jdi.ReferenceType;
4848
import com.sun.jdi.ThreadReference;
4949
import com.sun.jdi.Value;
50+
import com.sun.jdi.VMDisconnectedException;
5051
import com.sun.jdi.event.BreakpointEvent;
5152
import com.sun.jdi.event.Event;
5253
import com.sun.jdi.event.StepEvent;
@@ -241,11 +242,15 @@ public static boolean handleEvaluationResult(IDebugAdapterContext context, Threa
241242
if (resume) {
242243
return true;
243244
} else {
244-
if (ex != null) {
245-
logger.log(Level.SEVERE, String.format("[ConditionalBreakpoint]: %s", ex.getMessage() != null ? ex.getMessage() : ex.toString()), ex);
246-
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
247-
Events.UserNotificationEvent.NotificationType.ERROR,
248-
String.format("Breakpoint condition '%s' error: %s", breakpoint.getCondition(), ex.getMessage())));
245+
if (context.isVmTerminated()) {
246+
// do nothing
247+
} else if (ex != null) {
248+
if (!(ex instanceof VMDisconnectedException || ex.getCause() instanceof VMDisconnectedException)) {
249+
logger.log(Level.SEVERE, String.format("[ConditionalBreakpoint]: %s", ex.getMessage() != null ? ex.getMessage() : ex.toString()), ex);
250+
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
251+
Events.UserNotificationEvent.NotificationType.ERROR,
252+
String.format("Breakpoint condition '%s' error: %s", breakpoint.getCondition(), ex.getMessage())));
253+
}
249254
} else if (value == null || resultNotBoolean) {
250255
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
251256
Events.UserNotificationEvent.NotificationType.WARNING,

‎com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/eval/JdtEvaluationProvider.java‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,20 +148,12 @@ private CompletableFuture<Value> evaluate(String expression, ThreadReference thr
148148
ASTEvaluationEngine engine = new ASTEvaluationEngine(project, debugTarget);
149149
boolean newExpression = false;
150150
if (breakpoint != null) {
151-
if (StringUtils.isNotBlank(breakpoint.getLogMessage())) {
152-
compiledExpression = (ICompiledExpression) breakpoint.getCompiledLogpointExpression();
153-
if (compiledExpression == null) {
154-
newExpression = true;
155-
compiledExpression = engine.getCompiledExpression(expression, stackframe);
156-
breakpoint.setCompiledLogpointExpression(compiledExpression);
157-
}
158-
} else {
159-
compiledExpression = (ICompiledExpression) breakpoint.getCompiledConditionalExpression();
160-
if (compiledExpression == null) {
161-
newExpression = true;
162-
compiledExpression = engine.getCompiledExpression(expression, stackframe);
163-
breakpoint.setCompiledConditionalExpression(compiledExpression);
164-
}
151+
long threadId = thread.uniqueID();
152+
compiledExpression = (ICompiledExpression) breakpoint.getCompiledExpression(threadId);
153+
if (compiledExpression == null) {
154+
newExpression = true;
155+
compiledExpression = engine.getCompiledExpression(expression, stackframe);
156+
breakpoint.setCompiledExpression(threadId, compiledExpression);
165157
}
166158
} else {
167159
compiledExpression = engine.getCompiledExpression(expression, stackframe);

0 commit comments

Comments
 (0)