Skip to content

Commit

Permalink
Fix race competition when the breakpoint needs be evaluated in multi …
Browse files Browse the repository at this point in the history 
…threads (#325)

* Fix race competition when the breakpoint needs be evaluated in multi threads

Signed-off-by: Jinbo Wang <[email protected]>

* Use thread-safe map

Signed-off-by: Jinbo Wang <[email protected]>
  • Loading branch information
@testforstephen
testforstephen authored May 7, 2020
1 parent 74f5663 commit dacf440
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 25 deletions.
48 changes: 43 additions & 5 deletions ...ft.java.debug.core/src/main/java/com/microsoft/java/debug/core/EvaluatableBreakpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,39 @@

import org.apache.commons.lang3.StringUtils;

import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;

import com.sun.jdi.event.ThreadDeathEvent;
import com.sun.jdi.ThreadReference;
import com.sun.jdi.VirtualMachine;

import io.reactivex.disposables.Disposable;

public class EvaluatableBreakpoint extends Breakpoint implements IEvaluatableBreakpoint {
private IEventHub eventHub = null;
private Object compiledConditionalExpression = null;
private Object compiledLogpointExpression = null;
private Map<Long, Object> compiledExpressions = new ConcurrentHashMap<>();

EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber) {
super(vm, eventHub, className, lineNumber, 0, null);
this(vm, eventHub, className, lineNumber, 0, null);
}

EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount) {
super(vm, eventHub, className, lineNumber, hitCount, null);
this(vm, eventHub, className, lineNumber, hitCount, null);
}

EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount, String condition) {
super(vm, eventHub, className, lineNumber, hitCount, condition);
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount,
String condition) {
this(vm, eventHub, className, lineNumber, hitCount, condition, null);
}

EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount, String condition, String logMessage) {
EvaluatableBreakpoint(VirtualMachine vm, IEventHub eventHub, String className, int lineNumber, int hitCount,
String condition, String logMessage) {
super(vm, eventHub, className, lineNumber, hitCount, condition, logMessage);
this.eventHub = eventHub;
}

@Override
Expand Down Expand Up @@ -74,11 +87,36 @@ public Object getCompiledLogpointExpression() {
public void setCondition(String condition) {
super.setCondition(condition);
setCompiledConditionalExpression(null);
compiledExpressions.clear();
}

@Override
public void setLogMessage(String logMessage) {
super.setLogMessage(logMessage);
setCompiledLogpointExpression(null);
compiledExpressions.clear();
}

@Override
public Object getCompiledExpression(long threadId) {
return compiledExpressions.get(threadId);
}

@Override
public void setCompiledExpression(long threadId, Object compiledExpression) {
compiledExpressions.put(threadId, compiledExpression);
}

@Override
public CompletableFuture<IBreakpoint> install() {
Disposable subscription = eventHub.events()
.filter(debugEvent -> debugEvent.event instanceof ThreadDeathEvent)
.subscribe(debugEvent -> {
ThreadReference deathThread = ((ThreadDeathEvent) debugEvent.event).thread();
compiledExpressions.remove(deathThread.uniqueID());
});
super.subscriptions().add(subscription);

return super.install();
}
}
33 changes: 33 additions & 0 deletions ...t.java.debug.core/src/main/java/com/microsoft/java/debug/core/IEvaluatableBreakpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,44 @@ public interface IEvaluatableBreakpoint {

void setLogMessage(String logMessage);

/**
* please use {@link #setCompiledExpression(long, Object)} instead.
*/
@Deprecated
void setCompiledConditionalExpression(Object compiledExpression);

/**
* please use {@link #getCompiledExpression(long)} instead.
*/
@Deprecated
Object getCompiledConditionalExpression();

/**
* please use {@link #setCompiledExpression(long, Object)} instead.
*/
@Deprecated
void setCompiledLogpointExpression(Object compiledExpression);

/**
* please use {@link #getCompiledExpression(long)} instead.
*/
@Deprecated
Object getCompiledLogpointExpression();

/**
* Sets the compiled expression for a thread.
*
* @param threadId - thread the breakpoint is hit in
* @param compiledExpression - associated compiled expression
*/
void setCompiledExpression(long threadId, Object compiledExpression);

/**
* Returns existing compiled expression for the given thread or
* <code>null</code>.
*
* @param threadId thread the breakpoint was hit in
* @return compiled expression or <code>null</code>
*/
Object getCompiledExpression(long threadId);
}
26 changes: 25 additions & 1 deletion com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Watchpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,20 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;

import org.apache.commons.lang3.StringUtils;

import com.sun.jdi.Field;
import com.sun.jdi.ReferenceType;
import com.sun.jdi.ThreadReference;
import com.sun.jdi.VMDisconnectedException;
import com.sun.jdi.VirtualMachine;
import com.sun.jdi.event.ClassPrepareEvent;
import com.sun.jdi.event.ThreadDeathEvent;
import com.sun.jdi.request.ClassPrepareRequest;
import com.sun.jdi.request.EventRequest;
import com.sun.jdi.request.WatchpointRequest;
Expand All @@ -41,6 +45,7 @@ public class Watchpoint implements IWatchpoint, IEvaluatableBreakpoint {
private int hitCount;
private HashMap<Object, Object> propertyMap = new HashMap<>();
private Object compiledConditionalExpression = null;
private Map<Long, Object> compiledExpressions = new ConcurrentHashMap<>();

// IDebugResource
private List<EventRequest> requests = new ArrayList<>();
Expand Down Expand Up @@ -116,6 +121,7 @@ public String getCondition() {
public void setCondition(String condition) {
this.condition = condition;
setCompiledConditionalExpression(null);
compiledExpressions.clear();
}

@Override
Expand Down Expand Up @@ -147,6 +153,14 @@ public Object getProperty(Object key) {

@Override
public CompletableFuture<IWatchpoint> install() {
Disposable subscription = eventHub.events()
.filter(debugEvent -> debugEvent.event instanceof ThreadDeathEvent)
.subscribe(debugEvent -> {
ThreadReference deathThread = ((ThreadDeathEvent) debugEvent.event).thread();
compiledExpressions.remove(deathThread.uniqueID());
});
subscriptions.add(subscription);

// It's possible that different class loaders create new class with the same name.
// Here to listen to future class prepare events to handle such case.
ClassPrepareRequest classPrepareRequest = vm.eventRequestManager().createClassPrepareRequest();
Expand All @@ -155,7 +169,7 @@ public CompletableFuture<IWatchpoint> install() {
requests.add(classPrepareRequest);

CompletableFuture<IWatchpoint> future = new CompletableFuture<>();
Disposable subscription = eventHub.events()
subscription = eventHub.events()
.filter(debugEvent -> debugEvent.event instanceof ClassPrepareEvent && (classPrepareRequest.equals(debugEvent.event.request())))
.subscribe(debugEvent -> {
ClassPrepareEvent event = (ClassPrepareEvent) debugEvent.event;
Expand Down Expand Up @@ -249,4 +263,14 @@ public void setCompiledLogpointExpression(Object compiledExpression) {
public Object getCompiledLogpointExpression() {
return null;
}

@Override
public Object getCompiledExpression(long threadId) {
return compiledExpressions.get(threadId);
}

@Override
public void setCompiledExpression(long threadId, Object compiledExpression) {
compiledExpressions.put(threadId, compiledExpression);
}
}
1 change: 1 addition & 0 deletions .../java/com/microsoft/java/debug/core/adapter/handler/AbstractDisconnectRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public List<Command> getTargetCommands() {
@Override
public CompletableFuture<Response> handle(Command command, Arguments arguments, Response response,
IDebugAdapterContext context) {
context.setVmTerminated();
destroyDebugSession(command, arguments, response, context);
destroyResource(context);
return CompletableFuture.completedFuture(response);
Expand Down
15 changes: 10 additions & 5 deletions ...main/java/com/microsoft/java/debug/core/adapter/handler/SetBreakpointsRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.sun.jdi.ReferenceType;
import com.sun.jdi.ThreadReference;
import com.sun.jdi.Value;
import com.sun.jdi.VMDisconnectedException;
import com.sun.jdi.event.BreakpointEvent;
import com.sun.jdi.event.Event;
import com.sun.jdi.event.StepEvent;
Expand Down Expand Up @@ -241,11 +242,15 @@ public static boolean handleEvaluationResult(IDebugAdapterContext context, Threa
if (resume) {
return true;
} else {
if (ex != null) {
logger.log(Level.SEVERE, String.format("[ConditionalBreakpoint]: %s", ex.getMessage() != null ? ex.getMessage() : ex.toString()), ex);
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
Events.UserNotificationEvent.NotificationType.ERROR,
String.format("Breakpoint condition '%s' error: %s", breakpoint.getCondition(), ex.getMessage())));
if (context.isVmTerminated()) {
// do nothing
} else if (ex != null) {
if (!(ex instanceof VMDisconnectedException || ex.getCause() instanceof VMDisconnectedException)) {
logger.log(Level.SEVERE, String.format("[ConditionalBreakpoint]: %s", ex.getMessage() != null ? ex.getMessage() : ex.toString()), ex);
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
Events.UserNotificationEvent.NotificationType.ERROR,
String.format("Breakpoint condition '%s' error: %s", breakpoint.getCondition(), ex.getMessage())));
}
} else if (value == null || resultNotBoolean) {
context.getProtocolServer().sendEvent(new Events.UserNotificationEvent(
Events.UserNotificationEvent.NotificationType.WARNING,
Expand Down
20 changes: 6 additions & 14 deletions ...in/src/main/java/com/microsoft/java/debug/plugin/internal/eval/JdtEvaluationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,12 @@ private CompletableFuture<Value> evaluate(String expression, ThreadReference thr
ASTEvaluationEngine engine = new ASTEvaluationEngine(project, debugTarget);
boolean newExpression = false;
if (breakpoint != null) {
if (StringUtils.isNotBlank(breakpoint.getLogMessage())) {
compiledExpression = (ICompiledExpression) breakpoint.getCompiledLogpointExpression();
if (compiledExpression == null) {
newExpression = true;
compiledExpression = engine.getCompiledExpression(expression, stackframe);
breakpoint.setCompiledLogpointExpression(compiledExpression);
}
} else {
compiledExpression = (ICompiledExpression) breakpoint.getCompiledConditionalExpression();
if (compiledExpression == null) {
newExpression = true;
compiledExpression = engine.getCompiledExpression(expression, stackframe);
breakpoint.setCompiledConditionalExpression(compiledExpression);
}
long threadId = thread.uniqueID();
compiledExpression = (ICompiledExpression) breakpoint.getCompiledExpression(threadId);
if (compiledExpression == null) {
newExpression = true;
compiledExpression = engine.getCompiledExpression(expression, stackframe);
breakpoint.setCompiledExpression(threadId, compiledExpression);
}
} else {
compiledExpression = engine.getCompiledExpression(expression, stackframe);
Expand Down

0 comments on commit dacf440

Please sign in to comment.