Repository navigation
fix(worker): let a heap-limit terminate re-enter the inspector lock from console.log - #2070
adrian-niculescu wants to merge 1 commit into
Conversation
…rom 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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe inspector mutex changes from ChangesInspector Lock
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change allows worker termination to proceed when heap-limit handling re-enters the inspector lock, while continuing to serialize access from other threads. No material merge-blocking risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the lock with care Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
This branch has not been deployed
1 participant
In a debug build, a worker that runs out of heap inside
console.logcan leave its thread deadlocked for good: the parent still gets the heap-limit error, but the worker never finishes terminating.WorkerWrapper::ConsoleLogholdsinspectorMutex_acrossWorkerInspectorClient::consoleLog, which allocates on the worker heap: it captures a stack trace, and building the message text converts the arguments, which can run getters. When an allocation there reaches the worker's limit, V8 runsOnNearHeapLimitsynchronously on the same thread, which callsTerminate(), which takesinspectorMutex_again to notify the inspector. The mutex is not recursive, so the thread blocks on itself. This holds while no DevTools session has enabled the Debugger domain; once one has, V8 calls its own near-heap-limit callback instead of the worker's.inspectorMutex_is astd::recursive_mutexnow: the same thread re-enters, and every other thread is excluded as before. Releasing the mutex acrossconsoleLoginstead would not be safe, because a worker callback run through__runOnMainThreadlogs from the main thread while the worker thread can delete the client.With
OnNearHeapLimitcalled at that point by hand, the worker thread onmainstays blocked inTerminate()underConsoleLog, and with this change it finishes terminating. The only difference a spec could observe is a thread that never finishes, so there is none. The full device suite passes.Summary by CodeRabbit