Merge pull request #43573 from NipunaRanasinghe/debugger-variable-opt…

…imizations-u11

[Debugger] Improve loading time of debug variables and evaluation results

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/BreakpointProcessor.java

@@ -234,7 +234,9 @@ void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode, boolea
                 Location currentLocation = validFrames.get(0).getJStackFrame().location();
                 Optional<Location> prevLocation = context.getPrevLocation();
                 if (!validate || prevLocation.isEmpty() || !isWithinSameSource(currentLocation, prevLocation.get())) {
-                    doActivateDynamicBreakPoints(currentLocation);
+                    context.getEventManager().deleteAllBreakpoints();
+                    configureBreakpointsForMethod(currentLocation);
+                    context.setPrevLocation(currentLocation);
                 }
                 context.setPrevLocation(currentLocation);
             }
@@ -243,7 +245,11 @@ void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode, boolea
             // temporary breakpoint on the location of its invocation. This is supposed to handle the situations where
             // the user wants to step over on an exit point of the current function.
             if (mode == DynamicBreakpointMode.CALLER && validFrames.size() > 1) {
-                doActivateDynamicBreakPoints(validFrames.get(1).getJStackFrame().location());
+                context.getEventManager().deleteAllBreakpoints();
+                for (int frameIndex = 1; frameIndex < validFrames.size(); frameIndex++) {
+                    configureBreakpointsForMethod(validFrames.get(frameIndex).getJStackFrame().location());
+                }
+                context.setPrevLocation(validFrames.get(0).getJStackFrame().location());
             }
         } catch (JdiProxyException e) {
             LOGGER.error(e.getMessage());
@@ -254,12 +260,6 @@ void activateDynamicBreakPoints(int threadId, DynamicBreakpointMode mode, boolea
         }
     }
 
-    private void doActivateDynamicBreakPoints(Location location) {
-        context.getEventManager().deleteAllBreakpoints();
-        configureBreakpointsForMethod(location);
-        context.setPrevLocation(location);
-    }
-
     /**
      * Checks whether the given two locations are within the same source file.
      *

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/JDIEventProcessor.java

@@ -74,7 +74,7 @@ List<EventRequest> getStepRequests() {
     /**
      * Asynchronously listens and processes the incoming JDI events.
      */
-    void startListening() {
+    void listenAsync() {
         CompletableFuture.runAsync(() -> {
             isRemoteVmAttached = true;
             while (isRemoteVmAttached) {

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/evaluation/engine/invokable/RuntimeInstanceMethod.java

@@ -44,7 +44,7 @@ public RuntimeInstanceMethod(SuspendedContext context, Value objectRef, Method m
     }
 
     @Override
-    protected Value invoke() throws EvaluationException {
+    protected synchronized Value invoke() throws EvaluationException {
         try {
             if (!(objectValueRef instanceof ObjectReference objectReference)) {
                 throw createEvaluationException(FUNCTION_EXECUTION_ERROR, methodRef.name());

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/evaluation/engine/invokable/RuntimeStaticMethod.java

@@ -45,7 +45,7 @@ public RuntimeStaticMethod(SuspendedContext context, ReferenceType classRef, Met
     }
 
     @Override
-    protected Value invoke() throws EvaluationException {
+    protected synchronized Value invoke() throws EvaluationException {
         try {
             if (!(classRef instanceof ClassType classType)) {
                 throw createEvaluationException(FUNCTION_EXECUTION_ERROR, methodRef.name());

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/jdi/StackFrameProxyImpl.java

@@ -58,6 +58,7 @@ public class StackFrameProxyImpl extends JdiProxy implements StackFrameProxy {
     private ClassLoaderReference myClassLoader;
     private ThreeState myIsObsolete = ThreeState.UNSURE;
     private Map<LocalVariable, Value> myAllValues;
+    private static final int RETRY_COUNT = 20;
 
     public StackFrameProxyImpl(ThreadReferenceProxyImpl threadProxy, StackFrame frame, int fromBottomIndex) {
         super(threadProxy.getVirtualMachine());
@@ -72,7 +73,7 @@ public boolean isObsolete() throws JdiProxyException {
             return myIsObsolete.toBoolean();
         }
         InvalidStackFrameException error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 Method method = getMethod(location());
                 boolean isObsolete =
@@ -128,17 +129,31 @@ protected void clearCaches() {
     @Override
     public StackFrame getStackFrame() throws JdiProxyException {
         checkValid();
-        if (myStackFrame == null) {
+        if (myStackFrame != null) {
+            return myStackFrame;
+        }
+        IncompatibleThreadStateException error = null;
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 final ThreadReference threadRef = myThreadProxy.getThreadReference();
                 myStackFrame = threadRef.frame(getFrameIndex());
-            } catch (IndexOutOfBoundsException | IncompatibleThreadStateException e) {
+                return myStackFrame;
+            } catch (IncompatibleThreadStateException e) {
+                error = e;
+                clearCaches();
+                try {
+                    Thread.sleep(10);
+                } catch (InterruptedException ignored) {
+                }
+            } catch (IndexOutOfBoundsException e) {
                 throw new JdiProxyException(e.getMessage(), e);
             } catch (ObjectCollectedException ignored) {
                 throw new JdiProxyException("Thread has been collected");
+            } catch (Exception e) {
+                throw new JdiProxyException("Unknown error when trying to access thread reference", e);
             }
         }
-        return myStackFrame;
+        throw new JdiProxyException("Thread has become invalid", error);
     }
 
     @Override
@@ -162,7 +177,7 @@ public VirtualMachineProxyImpl getVirtualMachine() {
     @Override
     public Location location() throws JdiProxyException {
         InvalidStackFrameException error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 return getStackFrame().location();
             } catch (InvalidStackFrameException e) {
@@ -190,7 +205,7 @@ public String toString() {
     public ObjectReference thisObject() throws JdiProxyException {
         checkValid();
         try {
-            for (int attempt = 0; attempt < 2; attempt++) {
+            for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
                 try {
                     if (myThisReference == null) {
                         myThisReference = getStackFrame().thisObject();
@@ -221,7 +236,7 @@ public ObjectReference thisObject() throws JdiProxyException {
 
     public List<LocalVariableProxyImpl> visibleVariables() throws JdiProxyException {
         RuntimeException error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 final List<LocalVariable> list = getStackFrame().visibleVariables();
                 final List<LocalVariableProxyImpl> locals = new ArrayList<>(list.size());
@@ -252,7 +267,7 @@ public Value visibleValueByName(String name) throws JdiProxyException {
 
     protected LocalVariable visibleVariableByNameInt(String name) throws JdiProxyException {
         InvalidStackFrameException error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 try {
                     return getStackFrame().visibleVariableByName(name);
@@ -269,7 +284,7 @@ protected LocalVariable visibleVariableByNameInt(String name) throws JdiProxyExc
 
     public Value getValue(LocalVariableProxyImpl localVariable) throws JdiProxyException {
         Exception error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 Map<LocalVariable, Value> values = getAllValues();
                 LocalVariable variable = localVariable.getVariable();
@@ -333,7 +348,7 @@ private Map<LocalVariable, Value> getAllValues() throws JdiProxyException {
     public void setValue(LocalVariableProxyImpl localVariable, Value value)
             throws JdiProxyException, ClassNotLoadedException, InvalidTypeException {
         InvalidStackFrameException error = null;
-        for (int attempt = 0; attempt < 2; attempt++) {
+        for (int attempt = 0; attempt < RETRY_COUNT; attempt++) {
             try {
                 final LocalVariable variable = localVariable.getVariable();
                 final StackFrame stackFrame = getStackFrame();

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/variable/VariableUtils.java

@@ -173,20 +173,54 @@ public static String getStringFrom(Value stringValue) {
      * @return result of the method invocation as a string.
      */
     public static String getStringValue(SuspendedContext context, Value jvmObject) {
+        try {
+            Value result = invokeRemoteVMMethod(context, jvmObject, METHOD_STR_VALUE, Collections.singletonList(null));
+            return getStringFrom(result);
+        } catch (DebugVariableException e) {
+            return UNKNOWN_VALUE;
+        }
+    }
+
+    public static Value invokeRemoteVMMethod(SuspendedContext context, Value jvmObject, String methodName,
+                                             List<Value> arguments) throws DebugVariableException {
+        if (!(jvmObject instanceof ObjectReference)) {
+            throw new DebugVariableException("Failed to invoke remote VM method.");
+        }
+        Optional<Method> method = VariableUtils.getMethod(jvmObject, methodName);
+        if (method.isEmpty()) {
+            throw new DebugVariableException("Failed to invoke remote VM method.");
+        }
+
+        return invokeRemoteVMMethod(context, jvmObject, method.get(), arguments);
+    }
+
+    public static Value invokeRemoteVMMethod(SuspendedContext context, Value jvmObject, Method method,
+                                             List<Value> arguments) throws DebugVariableException {
         try {
             if (!(jvmObject instanceof ObjectReference)) {
-                return UNKNOWN_VALUE;
+                throw new DebugVariableException("Failed to invoke remote VM method.");
             }
-            Optional<Method> method = VariableUtils.getMethod(jvmObject, METHOD_STR_VALUE);
-            if (method.isPresent()) {
-                Value stringValue = ((ObjectReference) jvmObject).invokeMethod(context.getOwningThread()
-                                .getThreadReference(), method.get(), Collections.singletonList(null),
-                        ObjectReference.INVOKE_SINGLE_THREADED);
-                return VariableUtils.getStringFrom(stringValue);
+
+            if (arguments == null) {
+                arguments = Collections.emptyList();
             }
-            return UNKNOWN_VALUE;
-        } catch (Exception ignored) {
-            return UNKNOWN_VALUE;
+            // Since the remote VM's active thread will be accessed from multiple debugger threads, there's a chance
+            // for 'IncompatibleThreadStateException's. Therefore debugger might need to retry few times until the
+            // thread gets released. (current wait time => ~1000ms)
+            for (int attempt = 0; attempt < 100; attempt++) {
+                try {
+                    return ((ObjectReference) jvmObject).invokeMethod(context.getOwningThread()
+                            .getThreadReference(), method, arguments, ObjectReference.INVOKE_SINGLE_THREADED);
+                } catch (Exception e) {
+                    try {
+                        Thread.sleep(10);
+                    } catch (InterruptedException ignored) {
+                    }
+                }
+            }
+            throw new DebugVariableException("Failed to invoke remote VM method as the invocation thread is busy");
+        } catch (Exception e) {
+            throw new DebugVariableException("Failed to invoke remote VM method due to an internal error");
         }
     }
 

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/variable/types/BFuture.java

@@ -17,14 +17,12 @@
 package org.ballerinalang.debugadapter.variable.types;
 
 import com.sun.jdi.Method;
-import com.sun.jdi.ObjectReference;
 import com.sun.jdi.Value;
 import org.ballerinalang.debugadapter.SuspendedContext;
 import org.ballerinalang.debugadapter.variable.BVariableType;
 import org.ballerinalang.debugadapter.variable.NamedCompoundVariable;
 import org.ballerinalang.debugadapter.variable.VariableUtils;
 
-import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -68,9 +66,8 @@ public Map<String, Value> computeChildVariables() {
                 // Invokes "getLocalizedMessage()" method of the panic object.
                 Optional<Method> method = VariableUtils.getMethod(panic.get(), METHOD_LOCALIZEDMESSAGE);
                 if (method.isPresent()) {
-                    Value stringValue = ((ObjectReference) panic.get()).invokeMethod(getContext().getOwningThread()
-                                    .getThreadReference(), method.get(), new ArrayList<>(),
-                            ObjectReference.INVOKE_SINGLE_THREADED);
+                    Value stringValue = VariableUtils.invokeRemoteVMMethod(context, panic.get(),
+                            METHOD_LOCALIZEDMESSAGE, null);
                     childVarMap.put(FIELD_PANIC, stringValue);
                 }
             }

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/variable/types/BMap.java

@@ -19,7 +19,6 @@
 import com.sun.jdi.ArrayReference;
 import com.sun.jdi.IntegerValue;
 import com.sun.jdi.Method;
-import com.sun.jdi.ObjectReference;
 import com.sun.jdi.Value;
 import org.ballerinalang.debugadapter.SuspendedContext;
 import org.ballerinalang.debugadapter.variable.BVariableType;
@@ -113,12 +112,12 @@ private Map<Value, Value> getEntries(int startIndex, int count) {
 
     private Value getValueFor(Value key) {
         try {
-            Optional<Method> getMethod = VariableUtils.getMethod(jvmValue, METHOD_GET);
-            if (getMethod.isEmpty()) {
+            Optional<Method> getValueMethod = VariableUtils.getMethod(jvmValue, METHOD_GET);
+            if (getValueMethod.isEmpty()) {
                 return null;
             }
-            return ((ObjectReference) jvmValue).invokeMethod(context.getOwningThread().getThreadReference(),
-                    getMethod.get(), Collections.singletonList(key), ObjectReference.INVOKE_SINGLE_THREADED);
+            return VariableUtils.invokeRemoteVMMethod(context, jvmValue, getValueMethod.get(),
+                    Collections.singletonList(key));
         } catch (Exception ignored) {
             return null;
         }
@@ -130,9 +129,7 @@ private void loadAllKeys() {
             if (entrySetMethod.isEmpty()) {
                 return;
             }
-            Value keyArray = ((ObjectReference) jvmValue).invokeMethod(context.getOwningThread().getThreadReference(),
-                    entrySetMethod.get(), Collections.emptyList(), ObjectReference.INVOKE_SINGLE_THREADED);
-
+            Value keyArray = VariableUtils.invokeRemoteVMMethod(context, jvmValue, entrySetMethod.get(), null);
             loadedKeys = (ArrayReference) keyArray;
             loadedValues = new Value[getChildrenCount()];
         } catch (Exception ignored) {

misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/variable/types/BTable.java

@@ -144,8 +144,7 @@ private void populateTableSize() {
                 tableSize = -1;
                 return;
             }
-            Value size = ((ObjectReference) jvmValue).invokeMethod(getContext().getOwningThread().getThreadReference(),
-                    method.get(), new ArrayList<>(), ObjectReference.INVOKE_SINGLE_THREADED);
+            Value size = VariableUtils.invokeRemoteVMMethod(context, jvmValue, method.get(), null);
             tableSize = ((IntegerValue) size).intValue();
         } catch (Exception e) {
             this.tableSize = -1;
@@ -186,17 +185,15 @@ private Value getIterator() throws Exception {
         if (getIteratorMethod.isEmpty()) {
             return null;
         }
-        return ((ObjectReference) jvmValue).invokeMethod(getContext().getOwningThread().getThreadReference(),
-                getIteratorMethod.get(), new ArrayList<>(), ObjectReference.INVOKE_SINGLE_THREADED);
+        return VariableUtils.invokeRemoteVMMethod(context, jvmValue, getIteratorMethod.get(), null);
     }
 
     private boolean hasNext(Value iterator) throws Exception {
         Optional<Method> hasNextMethod = VariableUtils.getMethod(iterator, METHOD_HAS_NEXT);
         if (hasNextMethod.isEmpty()) {
             return false;
         }
-        Value hasNext = ((ObjectReference) iterator).invokeMethod(getContext().getOwningThread().getThreadReference(),
-                hasNextMethod.get(), new ArrayList<>(), ObjectReference.INVOKE_SINGLE_THREADED);
+        Value hasNext = VariableUtils.invokeRemoteVMMethod(context, iterator, hasNextMethod.get(), null);
         return Boolean.parseBoolean(hasNext.toString());
     }
 
@@ -205,16 +202,14 @@ private Value nextElement(Value iterator) throws Exception {
         if (nextMethod.isEmpty()) {
             return null;
         }
-        return ((ObjectReference) iterator).invokeMethod(getContext().getOwningThread().getThreadReference(),
-                nextMethod.get(), new ArrayList<>(), ObjectReference.INVOKE_SINGLE_THREADED);
+        return VariableUtils.invokeRemoteVMMethod(context, iterator, nextMethod.get(), null);
     }
 
     private Value getValues(Value next) throws Exception {
         Optional<Method> getValuesMethod = VariableUtils.getMethod(next, METHOD_GET_VALUES);
         if (getValuesMethod.isEmpty()) {
             return null;
         }
-        return ((ObjectReference) next).invokeMethod(getContext().getOwningThread().getThreadReference(),
-                getValuesMethod.get(), new ArrayList<>(), ObjectReference.INVOKE_SINGLE_THREADED);
+        return VariableUtils.invokeRemoteVMMethod(context, next, getValuesMethod.get(), null);
     }
 }

tests/jballerina-debugger-integration-test/src/main/java/org/ballerinalang/debugger/test/utils/DebugTestRunner.java

@@ -520,25 +520,26 @@ public boolean waitForDebugTermination(long timeoutMillis) {
      */
     public Map<String, Variable> fetchVariables(StoppedEventArguments args, VariableScope scope)
             throws BallerinaTestException {
-        Map<String, Variable> variables = new HashMap<>();
         if (!hitListener.getConnector().isConnected()) {
-            return variables;
+            throw new BallerinaTestException("Debug server is not connected.");
         }
-        StackTraceArguments stackTraceArgs = new StackTraceArguments();
+        StackTraceArguments traceArgs = new StackTraceArguments();
         VariablesArguments variableArgs = new VariablesArguments();
         ScopesArguments scopeArgs = new ScopesArguments();
-        stackTraceArgs.setThreadId(args.getThreadId());
+        traceArgs.setThreadId(args.getThreadId());
 
         try {
-            StackTraceResponse stackResp = hitListener.getConnector().getRequestManager().stackTrace(stackTraceArgs);
+            StackTraceResponse stackResp = hitListener.getConnector().getRequestManager().stackTrace(traceArgs);
             StackFrame[] stackFrames = stackResp.getStackFrames();
             if (stackFrames.length == 0) {
-                return variables;
+                throw new BallerinaTestException("Stack frame response does not contain any frames");
             }
             scopeArgs.setFrameId(scope == VariableScope.LOCAL ? stackFrames[0].getId() : -stackFrames[0].getId());
             ScopesResponse scopesResp = hitListener.getConnector().getRequestManager().scopes(scopeArgs);
             variableArgs.setVariablesReference(scopesResp.getScopes()[0].getVariablesReference());
             VariablesResponse variableResp = hitListener.getConnector().getRequestManager().variables(variableArgs);
+
+            Map<String, Variable> variables = new HashMap<>();
             Arrays.stream(variableResp.getVariables()).forEach(variable -> variables.put(variable.getName(), variable));
             return variables;
         } catch (Exception e) {