fix(ext/node): handle multiple calls in inspector.Session.post() (#31067)

Fixes a panic that occurred when using nested session.post() calls in
the node:inspector module. The issue manifested when callbacks from
inspector commands (like Profiler.enable) called session.post() again,
causing a re-entrant invocation of op_inspector_dispatch.

Fixes #31020

Author: fraidev

ext/node/ops/inspector.rs

@@ -167,17 +167,17 @@ where
   })
 }
 
-#[op2(fast)]
+#[op2(fast, reentrant)]
 pub fn op_inspector_dispatch(
-  #[cppgc] session: &JSInspectorSession,
+  #[cppgc] inspector: &JSInspectorSession,
   #[string] message: String,
 ) {
-  if let Some(session) = &mut *session.session.borrow_mut() {
+  if let Some(session) = &mut *inspector.session.borrow_mut() {
     session.dispatch(message);
   }
 }
 
 #[op2(fast)]
-pub fn op_inspector_disconnect(#[cppgc] session: &JSInspectorSession) {
-  drop(session.session.borrow_mut().take());
+pub fn op_inspector_disconnect(#[cppgc] inspector: &JSInspectorSession) {
+  inspector.session.borrow_mut().take();
 }

ext/node/polyfills/inspector.js

@@ -34,6 +34,8 @@ import {
 } from "ext:deno_node/internal/errors.ts";
 
 const {
+  ArrayPrototypePush,
+  ArrayPrototypeShift,
   SymbolDispose,
   JSONParse,
   JSONStringify,
@@ -44,12 +46,18 @@ class Session extends EventEmitter {
   #connection = null;
   #nextId = 1;
   #messageCallbacks = new SafeMap();
+  #pendingMessages = [];
+  #drainScheduled = false;
+  #isDraining = false;
 
   connect() {
     if (this.#connection) {
       throw new ERR_INSPECTOR_ALREADY_CONNECTED("The inspector session");
     }
-    this.#connection = op_inspector_connect(false, (m) => this.#onMessage(m));
+    this.#connection = op_inspector_connect(
+      false,
+      (m) => this.#enqueueMessage(m),
+    );
   }
 
   connectToMainThread() {
@@ -59,7 +67,10 @@ class Session extends EventEmitter {
     if (this.#connection) {
       throw new ERR_INSPECTOR_ALREADY_CONNECTED("The inspector session");
     }
-    this.#connection = op_inspector_connect(true, (m) => this.#onMessage(m));
+    this.#connection = op_inspector_connect(
+      true,
+      (m) => this.#enqueueMessage(m),
+    );
   }
 
   #onMessage(message) {
@@ -89,6 +100,28 @@ class Session extends EventEmitter {
     }
   }
 
+  #enqueueMessage(message) {
+    ArrayPrototypePush(this.#pendingMessages, message);
+    if (this.#isDraining) return;
+    if (!this.#drainScheduled) {
+      this.#drainScheduled = true;
+      process.nextTick(() => this.#drainMessages());
+    }
+  }
+
+  #drainMessages() {
+    this.#drainScheduled = false;
+    this.#isDraining = true;
+    try {
+      while (this.#pendingMessages.length > 0) {
+        const nextMessage = ArrayPrototypeShift(this.#pendingMessages);
+        this.#onMessage(nextMessage);
+      }
+    } finally {
+      this.#isDraining = false;
+    }
+  }
+
   post(method, params, callback) {
     validateString(method, "method");
     if (!callback && typeof params === "function") {
@@ -128,6 +161,9 @@ class Session extends EventEmitter {
     }
     this.#messageCallbacks.clear();
     this.#nextId = 1;
+    this.#pendingMessages.length = 0;
+    this.#drainScheduled = false;
+    this.#isDraining = false;
   }
 }
 

tests/unit_node/inspector_test.ts

@@ -3,7 +3,7 @@ import inspector, { Session } from "node:inspector";
 import inspectorPromises, {
   Session as SessionPromise,
 } from "node:inspector/promises";
-import { assertEquals } from "@std/assert/equals";
+import { assertEquals } from "@std/assert";
 
 Deno.test("[node/inspector] - importing inspector works", () => {
   assertEquals(typeof inspector.open, "function");
@@ -20,3 +20,77 @@ Deno.test("[node/inspector/promises] - importing inspector works", () => {
 Deno.test("[node/inspector/promises] - Session constructor should not throw", () => {
   new SessionPromise();
 });
+
+// Regression test for: https://github.com/denoland/deno/issues/31020
+Deno.test({
+  name: "[node/inspector] - deeply nested session.post() calls",
+  fn: async () => {
+    const session = new Session();
+    session.connect();
+
+    const results: number[] = [];
+
+    await new Promise<void>((resolve) => {
+      session.post("Profiler.enable", () => {
+        results.push(1);
+        session.post("Profiler.start", () => {
+          results.push(2);
+          session.post("Profiler.stop", () => {
+            results.push(3);
+            session.post("Profiler.disable", () => {
+              results.push(4);
+              session.disconnect();
+              resolve();
+            });
+          });
+        });
+      });
+    });
+
+    assertEquals(results, [1, 2, 3, 4]);
+  },
+});
+
+Deno.test({
+  name:
+    "[node/inspector] - multiple session.post() calls in same callback are queued",
+  fn: async () => {
+    const session = new Session();
+    session.connect();
+
+    const results: string[] = [];
+
+    await new Promise<void>((resolve) => {
+      session.post("Profiler.enable", () => {
+        results.push("enable-callback-start");
+
+        // Make multiple session.post() calls in the same callback
+        // These should be queued and processed in order
+        session.post("Profiler.start", () => {
+          results.push("start-callback");
+        });
+
+        session.post("Profiler.stop", () => {
+          results.push("stop-callback");
+        });
+
+        session.post("Profiler.disable", () => {
+          results.push("disable-callback");
+          session.disconnect();
+          resolve();
+        });
+
+        results.push("enable-callback-end");
+      });
+    });
+
+    // Verify the outer callback completes first, then queued messages are processed
+    assertEquals(results, [
+      "enable-callback-start",
+      "enable-callback-end",
+      "start-callback",
+      "stop-callback",
+      "disable-callback",
+    ]);
+  },
+});