[NathanWalker]

Split out of #89 at review request: GC delivery should not hold one lock shared by all runtimes. This fixes the delivery race with a lock per runtime.

Summary

A GC batch could reach a worker runtime while that runtime was being disposed:

1. The GC thread picked the worker from the subscriber map.
2. The worker thread unsubscribed and disposed its native Runtime.
3. The GC thread called notifyGc into the disposed runtime and crashed.

 GcMonitor (GC thread), per subscriber with a batch:
-  runtime.notifyGc(ids)
+  synchronized (subscriber) { if (!subscriber.unsubscribed) runtime.notifyGc(ids) }
 unsubscribe(runtime):
-  synchronized (GcListener) { subscribers.remove(runtime) }
+  subscriber = subscribers.remove(runtime)
+  synchronized (subscriber) { subscriber.unsubscribed = true }   // waits out a delivery in progress
 MemoryMonitor notifyGc():
-  synchronized (GcListener) { for each runtime: notifyGc() }
+  for each subscriber: synchronized (subscriber) { if (!unsubscribed) notifyGc() }

• After unsubscribe(runtime) returns, no GC thread is inside that runtime's notifyGc, and none will enter it.
• Per-runtime locking: a delivery into runtime A takes only A's lock, so starting or stopping worker B never waits behind it. There is still one GC thread, which delivers to runtimes one at a time, as before.
• No deadlock: delivery takes the subscriber lock, then the runtime's engine lock. So unsubscribe() must not be called from inside that runtime's JS scope, and the Javadoc says so. Every caller meets this:
  • worker shutdown calls detachWorkerRuntime after the worker loop has exited and before dispose;
  • the setup-failure rollbacks run after the native scope has closed;
  • fix(android): re-attach NativeScript after a JavaScript reload #89's detach() runs after React Native has stopped the JS thread.

Evidence

Tested on an emulator (API 36, arm64, debug build) with V8.

• Test suite: napi 522/522 and jsi 450/450, including the 27 TNS Workers tests.
• Stress test (not committed: it takes 90 s per run and depends on timing). 12 workers at a time are created and terminated, each churning Java objects and forcing JS and Java GC, while the main runtime forces GC every 25 ms.

Code	Runs	Result
Before, napi	3	2 crashed with SIGSEGV on the GC thread. Logs show the worker's unsubscribe and the GC thread entering notifyGc for the same runtime in the same millisecond, then the crash 8 ms later.
Before, jsi	2	The race did not reproduce.
After, napi + jsi	6	No crash over about 3,600 workers. 32 unsubscribes waited 1–5 ms for a delivery in progress, and 5 batches were skipped for unsubscribed runtimes.

• Unrelated: the stress test also found a separate memory error in ArgConverter::GetTypeLongCache when workers start at the same time, in both trees. It is out of scope here.
• Not verified: engines other than V8, and the React Native host-runtime path (fix(android): re-attach NativeScript after a JavaScript reload #89 isn't on this base).

Merge Danger

Door: two-way

Blast Radius: the GC notification path for every Android runtime (main and workers, both binding layers)

The change adds one rule: unsubscribe() must not be called from inside the runtime's JS scope. Every current caller follows it. A future caller that breaks it would deadlock that runtime against the GC thread.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d8cd64a7-6929-43ee-a1dd-ba7d65a3ebbb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · 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

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