From ff1f6242c7f4a6359a29349ede6fd11ceb5411b9 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:40:53 +0300 Subject: [PATCH 1/3] fix(worker): let a heap-limit terminate re-enter the inspector lock from console.log A worker's console.log holds inspectorMutex_ across the inspector's consoleLog, which captures a stack trace and so allocates. When that allocation hits the worker's heap cap, the near-heap-limit callback calls Terminate(), which takes the same non-recursive mutex on the same thread and deadlocks the worker. The mutex is recursive now, so that re-entry proceeds while other threads stay excluded as before. --- test-app/runtime/src/main/cpp/WorkerWrapper.cpp | 8 ++++---- test-app/runtime/src/main/cpp/WorkerWrapper.h | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp index 04abc68fd..f06b2c385 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp @@ -170,7 +170,7 @@ void WorkerWrapper::Terminate() { // A worker paused at a breakpoint sits in the inspector's nested pause // loop, not in Looper.loop() - kick it loose so TerminateExecution and // the looper quit below can take effect. - std::lock_guard lock(inspectorMutex_); + std::lock_guard lock(inspectorMutex_); if (inspector_ != nullptr) { inspector_->NotifyTerminating(); } @@ -810,7 +810,7 @@ void WorkerWrapper::CreateInspector(Isolate* isolate) { auto* client = new WorkerInspectorClient(workerId_, isolate, ALooper_forThread(), url); { - std::lock_guard lock(inspectorMutex_); + std::lock_guard lock(inspectorMutex_); inspector_ = client; } @@ -822,7 +822,7 @@ void WorkerWrapper::CreateInspector(Isolate* isolate) { void WorkerWrapper::DestroyInspector() { WorkerInspectorClient* client = nullptr; { - std::lock_guard lock(inspectorMutex_); + std::lock_guard lock(inspectorMutex_); client = inspector_; inspector_ = nullptr; } @@ -843,7 +843,7 @@ void WorkerWrapper::DestroyInspector() { void WorkerWrapper::ConsoleLog(v8_inspector::ConsoleAPIType method, const std::vector>& args) { - std::lock_guard lock(inspectorMutex_); + std::lock_guard lock(inspectorMutex_); if (inspector_ != nullptr) { inspector_->consoleLog(method, args); } diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.h b/test-app/runtime/src/main/cpp/WorkerWrapper.h index 5dd3b2fa8..0f3f351cf 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.h +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.h @@ -238,7 +238,10 @@ class WorkerWrapper : public std::enable_shared_from_this { void DestroyInspector(); WorkerInspectorClient* inspector_ = nullptr; - std::mutex inspectorMutex_; + // Recursive: ConsoleLog holds it across consoleLog, whose stack capture + // allocates, and a near-heap-limit callback raised by that allocation + // calls Terminate(), which takes it again on the same thread. + std::recursive_mutex inspectorMutex_; #endif static std::mutex registryMutex_; From b7cf7408dc785cc8a193bdbe9a5a31abec4c00ed Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 19:02:44 -0300 Subject: [PATCH 2/3] docs(worker): say which threads take the worker inspector lock and why it re-enters console.* reaches WorkerWrapper::ConsoleLog from the main thread too, when a worker callback runs there through __runOnMainThread, and argument stringification inside consoleLog can run a nested console call on the same thread. --- .../runtime/src/main/cpp/JsV8InspectorClient.cpp | 6 +++--- .../runtime/src/main/cpp/WorkerInspectorClient.h | 5 +++-- test-app/runtime/src/main/cpp/WorkerWrapper.h | 15 ++++++++++----- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp b/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp index d1ef7be83..5beb11a14 100644 --- a/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp +++ b/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp @@ -1134,9 +1134,9 @@ void JsV8InspectorClient::sendToFrontEndCallback(const v8::FunctionCallbackInfo< } void JsV8InspectorClient::consoleLogCallback(Isolate* isolate, ConsoleAPIType method, const std::vector>& args) { - // Worker isolates route to their own inspector (we're on the worker's - // thread here). Checked first so worker logging never lazily constructs - // the root client. + // Worker isolates route to their own inspector (from the worker thread, + // or the main thread under __runOnMainThread). Checked first so worker + // logging never lazily constructs the root client. WorkerWrapper* worker = WorkerWrapper::FromIsolate(isolate); if (worker != nullptr) { worker->ConsoleLog(method, args); diff --git a/test-app/runtime/src/main/cpp/WorkerInspectorClient.h b/test-app/runtime/src/main/cpp/WorkerInspectorClient.h index a3deb572a..6cfcee6b2 100644 --- a/test-app/runtime/src/main/cpp/WorkerInspectorClient.h +++ b/test-app/runtime/src/main/cpp/WorkerInspectorClient.h @@ -74,8 +74,9 @@ class WorkerInspectorClient final : public v8_inspector::V8InspectorClient, return runningPauseLoop_.load(std::memory_order_acquire); } - // Worker thread. Mirrors JsV8InspectorClient::consoleLogCallback for this - // isolate. + // Thread holding the worker isolate's Locker (the worker thread, or the + // main thread under __runOnMainThread). Mirrors + // JsV8InspectorClient::consoleLogCallback for this isolate. void consoleLog(v8_inspector::ConsoleAPIType method, const std::vector>& args); diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.h b/test-app/runtime/src/main/cpp/WorkerWrapper.h index 0f30cefbe..c130e6f90 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.h +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.h @@ -166,8 +166,10 @@ class WorkerWrapper : public std::enable_shared_from_this { #ifdef APPLICATION_IN_DEBUG /* - * Worker thread (console.* fires on the isolate's own thread). Forwards - * to the worker's inspector so the log reaches DevTools' worker target. + * Any thread holding the worker isolate's Locker: the worker thread, or + * the main thread while it runs a worker callback through + * __runOnMainThread. Forwards to the worker's inspector so the log + * reaches DevTools' worker target. */ void ConsoleLog(v8_inspector::ConsoleAPIType method, const std::vector>& args); @@ -253,9 +255,12 @@ class WorkerWrapper : public std::enable_shared_from_this { void DestroyInspector(); WorkerInspectorClient* inspector_ = nullptr; - // Recursive: ConsoleLog holds it across consoleLog, whose stack capture - // allocates, and a near-heap-limit callback raised by that allocation - // calls Terminate(), which takes it again on the same thread. + // Held across consoleLog, which allocates and runs user JS while it + // stringifies the arguments; releasing it there would let the worker + // thread delete the client under a __runOnMainThread caller. Recursive + // because that call re-enters on the same thread: a nested console call + // from a getter, or a near-heap-limit callback raised by an allocation, + // which calls Terminate(). std::recursive_mutex inspectorMutex_; #endif From e9c093b545e4adc10efc96e19e110109a9362f7f Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 19:02:44 -0300 Subject: [PATCH 3/3] test(worker): a console call nested in argument formatting does not deadlock the worker --- test-app/app/src/main/assets/app/mainpage.js | 1 + .../assets/app/tests/testWorkerNestedConsole.js | 15 +++++++++++++++ .../assets/app/tests/workerNestedConsoleWorker.js | 11 +++++++++++ 3 files changed, 27 insertions(+) create mode 100644 test-app/app/src/main/assets/app/tests/testWorkerNestedConsole.js create mode 100644 test-app/app/src/main/assets/app/tests/workerNestedConsoleWorker.js diff --git a/test-app/app/src/main/assets/app/mainpage.js b/test-app/app/src/main/assets/app/mainpage.js index 3996a5d75..e20563c01 100644 --- a/test-app/app/src/main/assets/app/mainpage.js +++ b/test-app/app/src/main/assets/app/mainpage.js @@ -34,6 +34,7 @@ require("./tests/testMultithreadedJavascript"); require("./tests/testWorkerTerminateDuringLoad"); require("./tests/testWorkerOptions"); require("./tests/testWorkerResourceLimits"); +require("./tests/testWorkerNestedConsole"); require("./tests/testMessaging"); require("./tests/testInterfaceDefaultMethods"); require("./tests/testInterfaceStaticMethods"); diff --git a/test-app/app/src/main/assets/app/tests/testWorkerNestedConsole.js b/test-app/app/src/main/assets/app/tests/testWorkerNestedConsole.js new file mode 100644 index 000000000..4baaa1697 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/testWorkerNestedConsole.js @@ -0,0 +1,15 @@ +describe("Worker nested console call", function () { + it("should keep running when formatting a logged argument logs again", function (done) { + var worker = new Worker("./workerNestedConsoleWorker.js"); + // A worker stuck in ConsoleLog holds the inspector lock that + // terminate() takes, so only the success path may terminate it. + worker.onmessage = function (e) { + expect(e.data).toBe("alive"); + worker.terminate(); + done(); + }; + worker.onerror = function (e) { + done.fail(e.message); + }; + }); +}); diff --git a/test-app/app/src/main/assets/app/tests/workerNestedConsoleWorker.js b/test-app/app/src/main/assets/app/tests/workerNestedConsoleWorker.js new file mode 100644 index 000000000..9eac26f53 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/workerNestedConsoleWorker.js @@ -0,0 +1,11 @@ +// The inspector reads Symbol.toStringTag while it formats a console argument, +// so this getter runs inside the outer console.log. +var arg = {}; +Object.defineProperty(arg, Symbol.toStringTag, { + get: function () { + console.log("nested"); + return "Nested"; + } +}); +console.log(arg); +postMessage("alive");