Skip to content

Commit 59edaa4

Browse files
committed
fix: keep the Worker object rooted until its thread ends, and report the end as nsworkerended
terminate() reset the Worker object's persistent and dropped the registry entry the moment it was called, while the thread was still winding down: the wrapper stopped being reachable from native before it had finished, and anything the worker had already queued on the parent's loop was discarded on arrival. The root now survives terminate(); it is released by the worker thread's own last act, which posts the end back to the parent's event loop. That post no longer only clears. On the parent's thread it dispatches the internal `nsworkerended` event on the Worker object and only then releases the persistent and the registry entry, so the end of a worker is observable from JS for the first time. The node:worker_threads shim listens for it, which is what lets 'exit' be emitted exactly once for a worker's own close() as much as for terminate(), and lets terminate() resolve at that point rather than off a microtask — after every message and error the worker had already sent. A parent that is itself tearing down clears its children directly and never delivers the notification, matching iOS. Android needed neither half of the iOS change's lifetime rework: the wrapper is shared_ptr-owned by the registry and by the detached thread itself, its poWorker_ has been a strong Persistent since construction, and the Worker object is a plain FunctionTemplate instance ObjectManager never sees — so there was no finalizer resurrection to take it off, and worker-thread posts already reached the parent through a weak_ptr to its event loop rather than through its isolate. Mirrors NativeScript/ios#456.
1 parent 42a8bcf commit 59edaa4

15 files changed

Lines changed: 516 additions & 31 deletions

docs/worker-threads.md

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ means deliberately unsupported.
5454
| `threadName` | shim | Always `undefined`. |
5555
| `workerData` | shim | Always `null` — see below. |
5656
| `parentPort` | shim | `null` on the main isolate. Inside a worker, a `MessagePort`-shaped `EventTarget` over the worker's existing parent channel: `postMessage` forwards to the global `postMessage`, `message`/`messageerror` are re-dispatched from the worker global scope, `start()` and `close()` are no-ops. It is **not** a real port: not transferable, no queue of its own. |
57-
| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. |
57+
| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. `exit` (always code `0`) fires exactly once, when the thread has ended, whether the worker was terminated or ended by its own `close()`; `terminate()` resolves at the same point. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. |
5858
| `postMessageToThread` | throws | `Error: postMessageToThread is not supported in this runtime`. |
5959
| `moveMessagePortToContext` | throws | `Error: moveMessagePortToContext is not supported in this runtime`. |
6060
| `locks` | absent | Web Locks are not implemented; the property does not exist. |
@@ -72,12 +72,16 @@ Values are cloned on the way in and deserialized fresh on each read, so
7272
mutating the object you passed does not reach a reader, and two readers never
7373
share one object.
7474

75-
### `exit` comes only from `terminate()`
75+
### `exit` fires when the thread has ended, always with code `0`
7676

77-
The runtime has no thread-exit signal — nothing reports that a worker's isolate
78-
finished. `terminate()` therefore resolves with `0` and emits `exit` with code
79-
`0` on the way, and that is the only path that emits it. A worker that ends by
80-
its own `close()` produces no `exit`.
77+
`exit` is emitted once, from the runtime's end-of-worker notification, so every
78+
`message` and `error` the worker produced before it ended has been delivered
79+
first. Node reports the thread's exit code; this runtime has none to report, so
80+
the code is `0` whichever way the worker ended — `terminate()`, its own
81+
`close()`, an uncaught error or a missing entry. `terminate()` resolves with
82+
`0` at the same moment `exit` fires. A parent that is itself tearing down never
83+
delivers the notification, so a `terminate()` awaited from a dying isolate
84+
stays pending, as it does in Node when the parent process exits.
8185

8286
### A worker error carries no `error` object, and the worker scope's `onerror` is not an event
8387

@@ -264,3 +268,40 @@ rather than raising a `DataCloneError`, which is long-standing behaviour app
264268
code relies on. Transfer is not part of that leniency — a port in a worker
265269
transfer list is validated exactly as it is everywhere else, since degrading a
266270
transfer would strand the port's sibling.
271+
272+
## Worker lifetime
273+
274+
**A `Worker` is held strongly by the runtime from the moment it is constructed
275+
until its thread ends**, the way a browser keeps a running worker's handle
276+
alive. Dropping every reference to one does not stop it: it keeps running, and
277+
it keeps dispatching `message` and `error` events at the handlers installed on
278+
it.
279+
280+
```js
281+
(function () {
282+
const worker = new Worker("./worker.js");
283+
worker.onmessage = handle; // still fires; nothing here holds `worker`
284+
worker.postMessage("go");
285+
})();
286+
```
287+
288+
Being a GC root also means a `Worker` is a well-behaved key: put one in a
289+
`WeakMap`, `WeakSet` or `WeakRef` and the entry survives for as long as the
290+
worker runs.
291+
292+
The root is released when the worker ends — `terminate()`, or the worker's own
293+
`close()`. `terminate()` only starts the wind-down: the object stays rooted
294+
until the worker thread has actually finished and reported that to the parent.
295+
From then on the object is collectable like any other, and the runtime drops
296+
the native side with it. Nothing about a *finished* worker is kept alive.
297+
298+
### `nsworkerended`
299+
300+
When the worker's thread has finished, the runtime dispatches a plain `Event`
301+
named `nsworkerended` on the `Worker` object. It is **internal and
302+
non-standard** — the web has no end-of-worker event, and the name is
303+
deliberately outside the standard namespace. It exists so that
304+
`node:worker_threads` can report `'exit'` for a worker that ended by its own
305+
`close()`; app code should not rely on it. The event is best effort: a worker
306+
whose parent is already tearing down never delivers it, because the parent's
307+
own teardown disposes the worker anyway.

test-app/app/src/main/assets/app/mainpage.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ require('./tests/testCreateRequire');
110110
require('./tests/testNodeUrlModule');
111111
require('./tests/testImportMetaResolution');
112112
require('./tests/testWorkerEsmEntry');
113+
// Worker wrapper reachability across GC (strong while running, collectable after)
114+
require('./tests/testWorkerLifetime');
113115
// Fetches from the in-app loopback fixture server, so it goes last
114116
require('./tests/testEsmHttpLoader');
115117
// Node-API addon surface
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
onmessage = function (event) {
2+
var port = event.data.port;
3+
postMessage(port, [port]);
4+
Atomics.store(event.data.flag, 0, 1);
5+
};
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Leaves a message that carries a port on this worker's own loop, undrained,
2+
// at the moment the parent terminates it: the port's sibling is port1, owned
3+
// by this worker. Spinning inside a timer callback keeps the loop from
4+
// draining while still letting terminate() interrupt the JS.
5+
var channel = new MessageChannel();
6+
var child = new Worker("./deadlockChild.js");
7+
var flag = new Int32Array(new SharedArrayBuffer(4));
8+
child.postMessage({ port: channel.port2, flag: flag }, [channel.port2]);
9+
setTimeout(function () {
10+
while (Atomics.load(flag, 0) === 0) {}
11+
postMessage("ready");
12+
for (;;) {}
13+
}, 0);

0 commit comments

Comments
 (0)