Skip to content

Commit 7e310d4

Browse files
committed
docs(runtime): drop the resurrection rationale the strong Worker root made stale, and name the real patch gate
1 parent 39f94e4 commit 7e310d4

3 files changed

Lines changed: 14 additions & 11 deletions

File tree

‎NativeScript/runtime/DataWrapper.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,8 +604,8 @@ class WorkerWrapper : public BaseDataWrapper {
604604
// ends, so a running worker is reachable the way a browser's is rather than
605605
// depending on its finalizer to keep it. Both of these run on the main
606606
// isolate's thread only -- they re-arm that isolate's global handle -- and
607-
// the unroot is idempotent, since terminate() and the thread-exit
608-
// notification can both reach it.
607+
// the unroot is idempotent, so an end reached by more than one path re-arms
608+
// the finalizer once.
609609
void RootWorkerObject();
610610
void UnrootWorkerObject();
611611
// Dispatches the end-of-worker event and unroots. Main isolate's thread,

‎NativeScript/runtime/js/events.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ function setListenerErrorReporter(fn) {
4444

4545
// Event name -> handler-attribute wrapper (see defineEventHandler), stored on
4646
// the target's own listener bag under a symbol so it cannot collide with an
47-
// event type. Deliberately NOT a WeakMap keyed by the target: a Worker is an
48-
// ObjectManager-registered object whose finalizer resurrects it while its
49-
// thread is alive, and a resurrected object's weak-collection entries are
50-
// already gone. Each wrapper carries a `delta` that the listener count is
51-
// corrected by: the wrapper occupies one slot in the listener list from its
52-
// first assignment onwards, but a cleared handler is not a listener.
47+
// event type. Deliberately NOT a WeakMap keyed by the target: the wrappers
48+
// live with the target, as Node keeps them, and stay independent of how the
49+
// collector treats weak-collection entries of objects that native code keeps
50+
// alive. Each wrapper carries a `delta` that the listener count is corrected
51+
// by: the wrapper occupies one slot in the listener list from its first
52+
// assignment onwards, but a cleared handler is not a listener.
5353
var kHandlers = Symbol("handlers");
5454

5555
function handlersOf(target) {

‎docs/knowledge/v8-resurrecting-finalizers.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,12 @@ default configuration reaches none of it.
240240
5. **Nested GC inside a finalizer callback.** Allocate heavily in the callback; confirm no
241241
double-invocation and no collection of the object under inspection.
242242

243-
The runtime's existing GC tests are the acceptance gate for the patch as the runtime uses it,
244-
and they pass — in particular *"Worker instance should not be garbage collected if the worker
245-
thread is alive"*, which exercises the `WorkerWrapper` resurrection site directly.
243+
`TestRunner/app/tests/GCFinalizerTests.js` is the acceptance gate for the patch as the runtime
244+
uses it. The Worker wrapper no longer depends on resurrection: a running worker's JS object is a
245+
strong root until its thread ends (`WorkerWrapper::RootWorkerObject`), so the shared test
246+
*"Worker instance should not be garbage collected if the worker thread is alive"* passes through
247+
rooting and never reaches the resurrection branch — it must not be read as evidence that a
248+
re-ported patch works. ObjectManager's refuse-and-re-weaken branch remains only as a fallback.
246249

247250
## Upgrade cost
248251

0 commit comments

Comments
 (0)