diff options
Diffstat (limited to 'src')
3 files changed, 19 insertions, 28 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto index 3d74f951af..d4cb453e99 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto @@ -192,17 +192,13 @@ message ThreadContinuedEvent { Thread thread = 1; } -// A location or condition where the debug server will pause execution. +// A location where the debug server will pause execution. message Breakpoint { oneof condition { // A breakpoint that is triggered when a particular line is reached. // Column index will be ignored for breakpoints. Location location = 1; } - // An optional condition for the breakpoint. When present, the breakpoint will - // be triggered iff both the primary condition holds and this expression - // evaluates to True. - string expression = 2; } // A single frame in a thread's stack trace. @@ -280,11 +276,6 @@ message ThreadPausedState { // The location in Skylark code of the next statement or expression that will // be executed. Location location = 2; - - // The list of stack frames for the paused thread. The first element in the - // list represents the topmost frame (that is, the current innermost - // function). - repeated Frame frame = 3; } // The reason why a thread was paused diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java index d1ccad25c6..4e9f69e1e8 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java @@ -96,6 +96,12 @@ final class ThreadHandler { private volatile ImmutableSet<SkylarkDebuggingProtos.Location> breakpoints = ImmutableSet.of(); /** + * True if the thread is currently performing a debugger-requested evaluation. If so, we don't + * check for breakpoints during the evaluation. + */ + private final ThreadLocal<Boolean> servicingEvalRequest = ThreadLocal.withInitial(() -> false); + + /** * Threads which are set to be paused in the next checked execution step. * * <p>Invariant: Every thread id in this set is also in {@link #threads}, provided that we are not @@ -198,6 +204,9 @@ final class ThreadHandler { } void pauseIfNecessary(Environment env, Location location, DebugServerTransport transport) { + if (servicingEvalRequest.get()) { + return; + } PauseReason pauseReason = shouldPauseCurrentThread(env, location); if (pauseReason != null) { pauseCurrentThread(env, pauseReason, location, transport); @@ -252,13 +261,17 @@ final class ThreadHandler { } debuggable = thread.debuggable; } - // no need to evaluate within the synchronize block: threads can only be resumed in response - // to a client request, and requests are handled serially + // no need to evaluate within the synchronize block: for paused threads, debuggable is only + // accessed in response to a client request, and requests are handled serially + // TODO(bazel-team): support asynchronous replies, and use finer-grained locks try { + servicingEvalRequest.set(true); Object result = debuggable.evaluate(expression); return DebuggerSerialization.getValueProto("Evaluation result", result); } catch (EvalException | InterruptedException e) { throw new DebugRequestException(e.getMessage()); + } finally { + servicingEvalRequest.set(false); } } @@ -344,8 +357,7 @@ final class ThreadHandler { builder.setThreadPausedState( ThreadPausedState.newBuilder() .setPauseReason(pausedState.pauseReason) - .setLocation(DebugEventHelper.getLocationProto(pausedState.location)) - .addAllFrame(listFrames(thread.debuggable, pausedState))); + .setLocation(DebugEventHelper.getLocationProto(pausedState.location))); } return builder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java index a0b4eefc43..f85a24ed8e 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java @@ -171,13 +171,7 @@ public class SkylarkDebugServerTest { .setThreadPausedState( ThreadPausedState.newBuilder() .setPauseReason(PauseReason.ALL_THREADS_PAUSED) - .setLocation(expectedLocation) - .addFrame( - Frame.newBuilder() - .setFunctionName("<top level>") - .setLocation(expectedLocation) - .addScope(Scope.newBuilder().setName("global")) - .build())) + .setLocation(expectedLocation)) .build()); sendStartDebuggingRequest(); @@ -217,12 +211,7 @@ public class SkylarkDebugServerTest { .setThreadPausedState( ThreadPausedState.newBuilder() .setPauseReason(PauseReason.HIT_BREAKPOINT) - .setLocation(breakpoint.toBuilder().setColumnNumber(1)) - .addFrame( - Frame.newBuilder() - .setFunctionName("<top level>") - .setLocation(breakpoint.toBuilder().setColumnNumber(1)) - .addScope(Scope.newBuilder().setName("global")))) + .setLocation(breakpoint.toBuilder().setColumnNumber(1))) .build(); assertThat(client.unnumberedEvents) @@ -449,7 +438,6 @@ public class SkylarkDebugServerTest { ThreadPausedState pausedState = threads.getThread(0).getThreadPausedState(); assertThat(pausedState.getPauseReason()).isEqualTo(PauseReason.STEPPING); assertThat(pausedState.getLocation()).isEqualTo(expectedLocation); - assertThat(pausedState.getFrameCount()).isEqualTo(2); } @Test |