diff --git a/src/bun.js/bindings/BunDebugger.cpp b/src/bun.js/bindings/BunDebugger.cpp index 0c0d6e97ab..200b2fd8e3 100644 --- a/src/bun.js/bindings/BunDebugger.cpp +++ b/src/bun.js/bindings/BunDebugger.cpp @@ -319,13 +319,24 @@ public: } } - for (auto* connection : connections) + for (auto* connection : connections) { connection->pauseFlags.store(0); + // Reset the scheduled flag so the debugger thread can post new + // tasks after the pause loop exits. + connection->jsThreadMessageScheduled.store(false); + } } void receiveMessagesOnInspectorThread(ScriptExecutionContext& context, Zig::GlobalObject* globalObject, bool connectIfNeeded) { - this->jsThreadMessageScheduledCount.store(0); + // Only clear the scheduled flag when NOT in the pause loop. + // During the pause loop, receiveMessagesOnInspectorThread is called + // repeatedly by the busy-poll. Clearing the flag would cause the + // debugger thread to re-post a task + interruptForMessageDelivery + // on every subsequent message, which is wasteful (and the posted + // tasks pile up for after the loop exits). + if (!(this->pauseFlags.load() & kInPauseLoop)) + this->jsThreadMessageScheduled.store(false); WTF::Vector messages; { @@ -365,7 +376,7 @@ public: void receiveMessagesOnDebuggerThread(ScriptExecutionContext& context, Zig::GlobalObject* debuggerGlobalObject) { - debuggerThreadMessageScheduledCount.store(0); + debuggerThreadMessageScheduled.store(false); WTF::Vector messages; { @@ -394,7 +405,7 @@ public: debuggerThreadMessages.append(inputMessage); } - if (this->debuggerThreadMessageScheduledCount++ == 0) { + if (!this->debuggerThreadMessageScheduled.exchange(true)) { debuggerScriptExecutionContext->postTaskConcurrently([connection = this](ScriptExecutionContext& context) { connection->receiveMessagesOnDebuggerThread(context, static_cast(context.jsGlobalObject())); }); @@ -407,10 +418,24 @@ public: Locker locker(jsThreadMessagesLock); jsThreadMessages.appendVector(inputMessages); } + scheduleInspectorThreadDelivery(); + } + void sendMessageToInspectorFromDebuggerThread(const WTF::String& inputMessage) + { + { + Locker locker(jsThreadMessagesLock); + jsThreadMessages.append(inputMessage); + } + scheduleInspectorThreadDelivery(); + } + +private: + void scheduleInspectorThreadDelivery() + { if (this->jsWaitForMessageFromInspectorLock.isLocked()) { this->jsWaitForMessageFromInspectorLock.unlock(); - } else if (this->jsThreadMessageScheduledCount++ == 0) { + } else if (!this->jsThreadMessageScheduled.exchange(true)) { ScriptExecutionContext::postTaskTo(scriptExecutionContextIdentifier, [connection = this](ScriptExecutionContext& context) { connection->receiveMessagesOnInspectorThread(context, static_cast(context.jsGlobalObject()), true); }); @@ -422,22 +447,7 @@ public: } } - void sendMessageToInspectorFromDebuggerThread(const WTF::String& inputMessage) - { - { - Locker locker(jsThreadMessagesLock); - jsThreadMessages.append(inputMessage); - } - - if (this->jsWaitForMessageFromInspectorLock.isLocked()) { - this->jsWaitForMessageFromInspectorLock.unlock(); - } else if (this->jsThreadMessageScheduledCount++ == 0) { - ScriptExecutionContext::postTaskTo(scriptExecutionContextIdentifier, [connection = this](ScriptExecutionContext& context) { - connection->receiveMessagesOnInspectorThread(context, static_cast(context.jsGlobalObject()), true); - }); - this->interruptForMessageDelivery(); - } - } +public: // Interrupt the JS thread to process pending CDP messages via StopTheWorld. // Only used on the SIGUSR1 runtime activation path where the event loop may @@ -459,11 +469,11 @@ public: WTF::Vector debuggerThreadMessages; WTF::Lock debuggerThreadMessagesLock = WTF::Lock(); - std::atomic debuggerThreadMessageScheduledCount { 0 }; + std::atomic debuggerThreadMessageScheduled { false }; WTF::Vector jsThreadMessages; WTF::Lock jsThreadMessagesLock = WTF::Lock(); - std::atomic jsThreadMessageScheduledCount { 0 }; + std::atomic jsThreadMessageScheduled { false }; JSC::JSGlobalObject* globalObject; ScriptExecutionContextIdentifier scriptExecutionContextIdentifier; diff --git a/test/js/bun/runtime-inspector/runtime-inspector.test.ts b/test/js/bun/runtime-inspector/runtime-inspector.test.ts index 0dded95d83..3a50e14758 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector.test.ts @@ -328,41 +328,19 @@ describe("Runtime inspector activation", () => { }); test.skipIf(skipASAN)("can interrupt an infinite loop", async () => { - using dir = tempDir("debug-infinite-loop-test", { - "target.js": ` - const fs = require("fs"); - const path = require("path"); - - // Write PID so parent can find us - fs.writeFileSync(path.join(process.cwd(), "pid"), String(process.pid)); - - // Infinite loop - the inspector should be able to interrupt this - while (true) {} - `, - }); - // Start target process with infinite loop await using targetProc = spawn({ - cmd: [bunExe(), "target.js"], - cwd: String(dir), + cmd: [bunExe(), "-e", `console.log(process.pid); while (true) {}`], env: bunEnv, stdout: "pipe", stderr: "pipe", }); - // Wait for PID file to be written - const pidPath = join(String(dir), "pid"); - let pid: number | undefined; - for (let i = 0; i < 50; i++) { - try { - const pidText = await Bun.file(pidPath).text(); - pid = parseInt(pidText, 10); - if (pid > 0) break; - } catch { - // File not ready yet - } - await Bun.sleep(100); - } + // Read PID from stdout (written before the infinite loop starts) + const reader = targetProc.stdout.getReader(); + const { value } = await reader.read(); + reader.releaseLock(); + const pid = parseInt(new TextDecoder().decode(value).trim(), 10); expect(pid).toBeGreaterThan(0); // Use _debugProcess to activate inspector - this should interrupt the infinite loop @@ -389,41 +367,19 @@ describe("Runtime inspector activation", () => { }); test.skipIf(skipASAN)("can pause execution during while(true) via CDP", async () => { - using dir = tempDir("debug-cdp-pause-test", { - "target.js": ` - const fs = require("fs"); - const path = require("path"); - - // Write PID so parent can find us - fs.writeFileSync(path.join(process.cwd(), "pid"), String(process.pid)); - - // Infinite loop - the debugger should be able to pause this via CDP - while (true) {} - `, - }); - // Start target process with infinite loop await using targetProc = spawn({ - cmd: [bunExe(), "target.js"], - cwd: String(dir), + cmd: [bunExe(), "-e", `console.log(process.pid); while (true) {}`], env: bunEnv, stdout: "pipe", stderr: "pipe", }); - // Wait for PID file to be written - const pidPath = join(String(dir), "pid"); - let pid: number | undefined; - for (let i = 0; i < 50; i++) { - try { - const pidText = await Bun.file(pidPath).text(); - pid = parseInt(pidText, 10); - if (pid > 0) break; - } catch { - // File not ready yet - } - await Bun.sleep(100); - } + // Read PID from stdout (written before the infinite loop starts) + const reader = targetProc.stdout.getReader(); + const { value } = await reader.read(); + reader.releaseLock(); + const pid = parseInt(new TextDecoder().decode(value).trim(), 10); expect(pid).toBeGreaterThan(0); // Activate inspector via _debugProcess