fix(worker): keep a worker isolate alive while a __runOnMainThread callback locks it

[adrian-niculescu]

A worker that calls __runOnMainThread(fn) and ends shortly after, for example through close(), can crash the app: the main thread locks the worker's isolate after the worker has disposed it, and dies with a SIGSEGV in v8::Locker::Initialize.

CallbackHandlers::RunMainThreadEntry reads the isolate from its cache entry, releases cacheMutex_, and only then constructs the v8::Locker. Worker teardown removes the entry in DestroyRuntime and disposes the isolate right after, so an entry the main thread already took can outlive its isolate. The main thread now records a hold on the isolate in the same cacheMutex_ section where it reads the entry, and drops it only after its Locker is released. The worker waits for those holds after it has released its own Locker and before it disposes the isolate. Waiting with the worker's Locker still held would deadlock against the main thread's Locker. A callback that took the entry before RemoveIsolateEntries ran then finds the entry gone under a live isolate and returns, and one that did not take it never sees the isolate.

A callback that throws has the same problem in another form: the exception kept a handle into the worker isolate after the hold was gone, and the event loop resolved that handle against the main thread's isolate when it reported it. For a worker's callback the exception now drops the handle while the isolate is still held, so the report carries its message and stack, as a failed runtime initialization already does. A callback on the main isolate keeps the handle, so its report is unchanged.

The window is a few instructions wide, so there is no spec for it. With a sleep injected between reading the entry and taking the Locker, a worker that closes right after posting its callback crashes the main thread on main and does not with this change. The full device suite passes.

Summary by CodeRabbit

• Bug Fixes
  • Improved worker shutdown reliability by waiting for pending main-thread callbacks to finish before disposing of the worker runtime.
  • Prevented callback activity from accessing an isolate during disposal.
  • Fixed cleanup of JavaScript handles when worker callbacks throw unhandled exceptions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a85a81dc-a1f2-4474-a08e-478e168b15f9

📥 Commits

Reviewing files that changed from the base of the PR and between 9b12329 and a340e10.

📒 Files selected for processing (4)

• test-app/runtime/src/main/cpp/CallbackHandlers.cpp
• test-app/runtime/src/main/cpp/CallbackHandlers.h
• test-app/runtime/src/main/cpp/Runtime.cpp
• test-app/runtime/src/main/cpp/WorkerWrapper.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

CallbackHandlers tracks active main-thread callbacks by isolate and provides a wait method. BackgroundLooper waits for callbacks after destroying the worker runtime and before disposing its isolate. Worker callback exception handling releases its JavaScript handle before rethrowing.

Changes

Worker callback lifecycle

Layer / File(s)	Summary
Track callback holds test-app/runtime/src/main/cpp/CallbackHandlers.h, test-app/runtime/src/main/cpp/CallbackHandlers.cpp	CallbackHandlers tracks active callback holds per isolate and adds WaitForMainThreadCallbacks. For worker runtimes, the callback exception path releases the JavaScript handle before throwing the exception onward.
Wait before isolate disposal test-app/runtime/src/main/cpp/WorkerWrapper.cpp, test-app/runtime/src/main/cpp/Runtime.cpp	BackgroundLooper waits for main-thread callbacks after DestroyRuntime() and before isolate->Dispose(). The Runtime.cpp comment describes the removal of untaken entries and waiting for entries already taken.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant BackgroundLooper
  participant Runtime
  participant CallbackHandlers
  participant MainThreadCallback
  participant Isolate
  BackgroundLooper->>Runtime: DestroyRuntime()
  Runtime->>CallbackHandlers: RemoveIsolateEntries()
  BackgroundLooper->>CallbackHandlers: WaitForMainThreadCallbacks(isolate)
  MainThreadCallback->>CallbackHandlers: RunMainThreadEntry()
  CallbackHandlers->>Isolate: Hold isolate while callback runs
  CallbackHandlers-->>CallbackHandlers: Release hold after isolate locker
  CallbackHandlers-->>BackgroundLooper: Return when no holds remain
  BackgroundLooper->>Isolate: Dispose()

Loading

Suggested reviewers: edusperoni

Merge Risk: ⚪ Minimal · up to a340e

No merge-blocking issue is established; the worker callback shutdown change is ready for normal merge checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: keeping a worker isolate alive while a __runOnMainThread callback acquires its locker.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit counts each callback’s stay
Then waits until the holds give way
The worker rests; its tasks are through
The isolate waits for the all-clear too
A carrot marks the shutdown queue}

_{Comment @coderabbitai help to get the list of available commands.}