PR2 of conditional BP (#154)

* Add basic impl of conditional BP

* remove an empty line.

* change a better var name

* add cache for conditional breakpoint to avoid duplicate compilation

* 1. fix an issue when exception is thrown during evaluation, the program is stopped at the exception
2. update a comment

* 1. fix the issue findFirst will throw exception if none is found.
2. fix the issue of a dirty evaluation environment cause the next evaluation failure.

* user filter to filter breakpoint event only

* rollback unfinished changes.

* 1. make ensureDebugTarget void and use exception for the error handling
2. keep only one CompletableFuture in evaluate

Author: andxu

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Breakpoint.java

@@ -127,6 +127,11 @@ public void setHitCount(int hitCount) {
             });
     }
 
+    @Override
+    public void setCondition(String condition) {
+        this.condition = condition;
+    }
+
     @Override
     public CompletableFuture<IBreakpoint> install() {
         // It's possible that different class loaders create new class with the same name.

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/IBreakpoint.java

@@ -29,4 +29,6 @@ public interface IBreakpoint extends IDebugResource {
     Object getProperty(Object key);
 
     String getCondition();
+
+    void setCondition(String condition);
 }

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/BreakpointManager.java

@@ -31,6 +31,9 @@ public class BreakpointManager {
     private List<IBreakpoint> breakpoints;
     private HashMap<String, HashMap<String, IBreakpoint>> sourceToBreakpoints;
     private AtomicInteger nextBreakpointId = new AtomicInteger(1);
+    // BreakpointManager is the owner class of the breakpoint to compiled expression map, it will remove
+    // the breakpoint from this map if the breakpoint is removed or its condition is changed
+    private Map<IBreakpoint, Object> breakpointExpressionMap = new HashMap<>();
 
     /**
      * Constructor.
@@ -77,6 +80,7 @@ public IBreakpoint[] setBreakpoints(String source, IBreakpoint[] breakpoints, bo
                 } catch (Exception e) {
                     logger.log(Level.SEVERE, String.format("Remove breakpoint exception: %s", e.toString()), e);
                 }
+                breakpointExpressionMap.remove(bp);
                 this.breakpoints.remove(bp);
             }
             this.sourceToBreakpoints.put(source, null);
@@ -143,6 +147,7 @@ private void removeBreakpointsInternally(String source, IBreakpoint[] breakpoint
                     // Destroy the breakpoint on the debugee VM.
                     breakpoint.close();
                     this.breakpoints.remove(breakpoint);
+                    breakpointExpressionMap.remove(breakpoint);
                     breakpointMap.remove(String.valueOf(breakpoint.getLineNumber()));
                 } catch (Exception e) {
                     logger.log(Level.SEVERE, String.format("Remove breakpoint exception: %s", e.toString()), e);
@@ -166,6 +171,28 @@ public IBreakpoint[] getBreakpoints(String source) {
         return breakpointMap.values().toArray(new IBreakpoint[0]);
     }
 
+
+    /**
+     * Get the compiled expression map with breakpoint, it will be used in JdtEvaluationProvider#evaluateForBreakpoint for storing
+     * the compiled expression when the first time this conditional breakpoint is hit.
+     *
+     * @return the compiled expression map
+     */
+    public Map<IBreakpoint, Object> getBreakpointExpressionMap() {
+        return breakpointExpressionMap;
+    }
+
+    /**
+     *  Update the condition for the specified breakpoint, and clear the compiled expression for the breakpoint.
+     *
+     * @param breakpoint the conditional breakpoint
+     * @param newCondition the new condition to be used.
+     */
+    public void updateConditionCompiledExpression(IBreakpoint breakpoint, String newCondition) {
+        breakpoint.setCondition(newCondition);
+        breakpointExpressionMap.remove(breakpoint);
+    }
+
     /**
      * Cleanup all breakpoints and reset the breakpoint id counter.
      */

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/IEvaluationProvider.java

@@ -11,8 +11,10 @@
 
 package com.microsoft.java.debug.core.adapter;
 
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
+import com.microsoft.java.debug.core.IBreakpoint;
 import com.sun.jdi.ThreadReference;
 import com.sun.jdi.Value;
 
@@ -30,7 +32,7 @@ public interface IEvaluationProvider extends IProvider {
     boolean isInEvaluation(ThreadReference thread);
 
     /**
-     * Evaluate the expression at the given project and thread and stack frame depth, the promise is to be resolved/rejected when
+     * Evaluate the expression at the given thread and stack frame depth, return the promise which is to be resolved/rejected when
      * the evaluation finishes.
      *
      * @param expression The expression to be evaluated
@@ -40,6 +42,19 @@ public interface IEvaluationProvider extends IProvider {
      */
     CompletableFuture<Value> evaluate(String expression, ThreadReference thread, int depth);
 
+    /**
+     * Evaluate the conditional breakpoint at the given thread and return the promise which is to be resolved/rejected when
+     * the evaluation finishes. The breakpointExpressionMap value should be managed by this IEvaluationProvider, avoid duplicate compilation
+     * on the same query when the conditional breakpoint is set inside a large loop, when the breakpoint is removed or the condition is changed,
+     * the external owner of breakpointExpressionMap must remove the related map entry.
+     *
+     * @param breakpoint The conditional breakpoint
+     * @param thread The jdi thread to the expression will be executed at
+     * @param breakpointExpressionMap The map has breakpoint as the key and the compiled expression object for next evaluation use.
+     * @return the evaluation result future
+     */
+    CompletableFuture<Value> evaluateForBreakpoint(IBreakpoint breakpoint, ThreadReference thread, Map<IBreakpoint, Object> breakpointExpressionMap);
+
 
     /**
      * Call this method when the thread is to be resumed by user, it will first cancel ongoing evaluation tasks on specified thread and
@@ -48,5 +63,4 @@ public interface IEvaluationProvider extends IProvider {
      * @param thread the JDI thread reference where the evaluation task is executing at
      */
     void clearState(ThreadReference thread);
-
 }

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/ConfigurationDoneRequestHandler.java

@@ -23,6 +23,7 @@
 import com.microsoft.java.debug.core.adapter.ErrorCode;
 import com.microsoft.java.debug.core.adapter.IDebugAdapterContext;
 import com.microsoft.java.debug.core.adapter.IDebugRequestHandler;
+import com.microsoft.java.debug.core.adapter.IEvaluationProvider;
 import com.microsoft.java.debug.core.protocol.Events;
 import com.microsoft.java.debug.core.protocol.Messages.Response;
 import com.microsoft.java.debug.core.protocol.Requests.Arguments;
@@ -94,6 +95,11 @@ private void handleDebugEvent(DebugEvent debugEvent, IDebugSession debugSession,
             // ignore since SetBreakpointsRequestHandler has already handled
         } else if (event instanceof ExceptionEvent) {
             ThreadReference thread = ((ExceptionEvent) event).thread();
+            ThreadReference bpThread = ((BreakpointEvent) event).thread();
+            IEvaluationProvider engine = context.getProvider(IEvaluationProvider.class);
+            if (engine.isInEvaluation(bpThread)) {
+                return;
+            }
             context.getProtocolServer().sendEvent(new Events.StoppedEvent("exception", thread.uniqueID()));
             debugEvent.shouldResume = false;
         } else {

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

@@ -15,7 +15,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -42,9 +41,11 @@
 import com.microsoft.java.debug.core.protocol.Requests.SetBreakpointArguments;
 import com.microsoft.java.debug.core.protocol.Responses;
 import com.microsoft.java.debug.core.protocol.Types;
-import com.sun.jdi.PrimitiveValue;
+import com.sun.jdi.BooleanValue;
+import com.sun.jdi.Field;
+import com.sun.jdi.ObjectReference;
+import com.sun.jdi.ReferenceType;
 import com.sun.jdi.ThreadReference;
-import com.sun.jdi.Value;
 import com.sun.jdi.event.BreakpointEvent;
 import com.sun.jdi.event.Event;
 import com.sun.jdi.event.StepEvent;
@@ -131,9 +132,16 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
                         Events.BreakpointEvent bpEvent = new Events.BreakpointEvent("new", this.convertDebuggerBreakpointToClient(bp, context));
                         context.getProtocolServer().sendEvent(bpEvent);
                     });
-                } else if (toAdds[i].getHitCount() != added[i].getHitCount() && added[i].className() != null) {
-                    // Update hitCount condition.
-                    added[i].setHitCount(toAdds[i].getHitCount());
+                } else if (added[i].className() != null) {
+                    if (toAdds[i].getHitCount() != added[i].getHitCount()) {
+                        // Update hitCount condition.
+                        added[i].setHitCount(toAdds[i].getHitCount());
+                    }
+
+                    if (!StringUtils.equals(toAdds[i].getCondition(), added[i].getCondition())) {
+                        manager.updateConditionCompiledExpression(added[i], toAdds[i].getCondition());
+                    }
+
                 }
                 res.add(this.convertDebuggerBreakpointToClient(added[i], context));
             }
@@ -149,10 +157,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
     private void registerBreakpointHandler(IDebugAdapterContext context) {
         IDebugSession debugSession = context.getDebugSession();
         if (debugSession != null) {
-            debugSession.getEventHub().events().subscribe(debugEvent -> {
-                if (!(debugEvent.event instanceof BreakpointEvent)) {
-                    return;
-                }
+            debugSession.getEventHub().events().filter(debugEvent -> debugEvent.event instanceof BreakpointEvent).subscribe(debugEvent -> {
                 Event event = debugEvent.event;
                 if (debugEvent.eventSet.size() > 1 && debugEvent.eventSet.stream().anyMatch(t -> t instanceof StepEvent)) {
                     // The StepEvent and BreakpointEvent are grouped in the same event set only if they occurs at the same location and in the same thread.
@@ -167,23 +172,30 @@ private void registerBreakpointHandler(IDebugAdapterContext context) {
                     // find the breakpoint related to this breakpoint event
                     IBreakpoint conditionalBP = Arrays.asList(manager.getBreakpoints()).stream().filter(bp -> StringUtils.isNotBlank(bp.getCondition())
                             && bp.requests().contains(((BreakpointEvent) event).request())
-                            ).findFirst().get();
+                            ).findFirst().orElse(null);
                     if (conditionalBP != null) {
                         CompletableFuture.runAsync(() -> {
-                            Value value;
-                            try {
-                                value = engine.evaluate(conditionalBP.getCondition(), bpThread, 0).get();
-                                if (value instanceof PrimitiveValue) {
-                                    boolean evaluationResultAsBool = ((PrimitiveValue) value).booleanValue();
-                                    if (!evaluationResultAsBool) {
-                                        debugEvent.eventSet.resume();
-                                        return;
+                            engine.evaluateForBreakpoint(conditionalBP, bpThread, manager.getBreakpointExpressionMap()).whenComplete((value, ex) -> {
+                                // TODO, notify user when error is raised.
+                                boolean resume = false;
+                                if (value != null && ex == null) {
+                                    if (value instanceof BooleanValue) {
+                                        resume = !((BooleanValue) value).booleanValue();
+                                    } else if (value instanceof ObjectReference
+                                            && ((ObjectReference) value).type().name().equals("java.lang.Boolean")) {
+                                        // get boolean value from java.lang.Boolean object
+                                        Field field = ((ReferenceType) ((ObjectReference) value).type()).fieldByName("value");
+                                        resume = !((BooleanValue) ((ObjectReference) value).getValue(field)).booleanValue();
                                     }
                                 }
-                            } catch (InterruptedException | ExecutionException e) {
-                                // TODO: notify user about evaluation failure
-                            }
-                            context.getProtocolServer().sendEvent(new Events.StoppedEvent("breakpoint", bpThread.uniqueID()));
+                                if (resume) {
+                                    debugEvent.eventSet.resume();
+                                    // since the evaluation result is false, clear the evaluation environment caused by above evaluation.
+                                    engine.clearState(bpThread);
+                                } else {
+                                    context.getProtocolServer().sendEvent(new Events.StoppedEvent("breakpoint", bpThread.uniqueID()));
+                                }
+                            });
 
                         });
                     } else {