Distinguish between threads paused during initialization, and threads paused in response to a explicit PauseThreadsRequest.
Makes client-side synchronization much easier.
This leaves b/112848203 unfixed for now -- I'll look at that separately
PiperOrigin-RevId: 209487111
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 a3df9e4..d3d1719 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
@@ -284,25 +284,29 @@
Error conditional_breakpoint_error = 5;
}
-// The reason why a thread was paused
+// The reason why a thread was paused.
enum PauseReason {
- // The debug server hasn't set a reason for pausing a thread.
+ // The debug server hasn't set any reason.
UNSET = 0;
// The stepping condition in a ContinueExecutionRequest was hit.
STEPPING = 1;
- // All threads are paused (e.g. prior to the initial StartDebuggingRequest).
+ // A PauseThreadRequest was sent with thread_id=0.
ALL_THREADS_PAUSED = 2;
- // Thread paused in response to a PauseThreadRequest.
+ // A PauseThreadRequest was sent with thread_id matching this thread.
PAUSE_THREAD_REQUEST = 3;
// A breakpoint was hit.
HIT_BREAKPOINT = 4;
- // An error occurred evaluating a breakpoint condition
+ // An error occurred while evaluating a breakpoint condition.
CONDITIONAL_BREAKPOINT_ERROR = 5;
+
+ // Debugging just started, and a StartDebuggingRequest has not yet been
+ // received and processed.
+ INITIALIZING = 6;
}
// The debugger representation of a Skylark value.
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 b2828f8..7675b52 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
@@ -77,13 +77,15 @@
}
}
- /**
- * If true, all threads will pause at the earliest possible opportunity. New threads will also be
- * immediately paused upon creation.
- *
- * <p>The debugger starts with all threads paused, until a StartDebuggingRequest is received.
- */
- private volatile boolean pausingAllThreads = true;
+ /** Whether threads are globally paused, and if so, why. */
+ private enum DebuggerState {
+ INITIALIZING, // no StartDebuggingRequest has yet been received; all threads are paused
+ ALL_THREADS_PAUSED, // all threads are paused in response to a PauseThreadRequest with id=0
+ RUNNING, // normal running: threads are not globally paused
+ }
+
+ /** The debugger starts with all threads paused, until a StartDebuggingRequest is received. */
+ private volatile DebuggerState debuggerState = DebuggerState.INITIALIZING;
/** A map from identifiers of paused threads to their state info. */
@GuardedBy("this")
@@ -114,7 +116,7 @@
/** Mark all current and future threads paused. Will take effect asynchronously. */
void pauseAllThreads() {
- pausingAllThreads = true;
+ debuggerState = DebuggerState.ALL_THREADS_PAUSED;
}
/** Mark the given thread paused. Will take effect asynchronously. */
@@ -152,7 +154,7 @@
*/
void resumeAllThreads() {
threadsToPause.clear();
- pausingAllThreads = false;
+ debuggerState = DebuggerState.RUNNING;
synchronized (this) {
for (PausedThreadState thread : ImmutableList.copyOf(pausedThreads.values())) {
// continue-all doesn't support stepping.
@@ -163,14 +165,14 @@
}
/**
- * Unpauses the given thread if it is currently paused. Also unsets {@link #pausingAllThreads}. If
- * the thread is not paused, but currently stepping, it clears the stepping behavior so it will
- * run unconditionally.
+ * Unpauses the given thread if it is currently paused. Also sets {@link #debuggerState} to
+ * RUNNING. If the thread is not paused, but currently stepping, it clears the stepping behavior
+ * so it will run unconditionally.
*/
void resumeThread(long threadId, SkylarkDebuggingProtos.Stepping stepping)
throws DebugRequestException {
// once the user has requested any thread be resumed, don't continue pausing future threads
- pausingAllThreads = false;
+ debuggerState = DebuggerState.RUNNING;
synchronized (this) {
threadsToPause.remove(threadId);
if (steppingThreads.remove(threadId) != null) {
@@ -326,9 +328,13 @@
private PauseReason shouldPauseCurrentThread(Environment env, Location location)
throws ConditionalBreakpointException {
long threadId = Thread.currentThread().getId();
- if (pausingAllThreads) {
+ DebuggerState state = debuggerState;
+ if (state == DebuggerState.ALL_THREADS_PAUSED) {
return PauseReason.ALL_THREADS_PAUSED;
}
+ if (state == DebuggerState.INITIALIZING) {
+ return PauseReason.INITIALIZING;
+ }
if (threadsToPause.contains(threadId)) {
return PauseReason.PAUSE_THREAD_REQUEST;
}
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 b1cf3d3..6275d22 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
@@ -155,7 +155,7 @@
SkylarkDebuggingProtos.PausedThread.newBuilder()
.setId(threadId)
.setName(threadName)
- .setPauseReason(PauseReason.ALL_THREADS_PAUSED)
+ .setPauseReason(PauseReason.INITIALIZING)
.setLocation(expectedLocation)
.build()));
