GeezerVison is showing github.com — esm: do not use `'beforeExit'` on the main thread · nodejs/node@1ec640a
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/hooks.js +4-17Lines changed: 4 additions & 17 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -521,14 +521,6 @@ class HooksProxy { | 521 | 521 | } | 522 | 522 | } | 523 | 523 |
| 524 | | - #beforeExitHandler = () => { | 525 | | - debug('beforeExit main thread', this.#lock, this.#numberOfPendingAsyncResponses); | 526 | | - if (this.#numberOfPendingAsyncResponses !== 0) { | 527 | | - // The worker still has some work to do, let's wait for it before terminating the process. | 528 | | - this.#worker.ref(); | 529 | | - } | 530 | | - }; | 531 | | - | 532 | 524 | async makeAsyncRequest(method, ...args) { | 533 | 525 | this.#waitForWorker(); | 534 | 526 |
| @@ -544,11 +536,9 @@ class HooksProxy { | 544 | 536 | // come AFTER the last task in the event loop has run its course and there would be nothing | 545 | 537 | // left keeping the thread alive (and once the main thread dies, the whole process stops). | 546 | 538 | // However we want to keep the process alive until the worker thread responds (or until the | 547 | | - // event loop of the worker thread is also empty). So we add the beforeExit handler whose | 548 | | - // mission is to lock the main thread until we hear back from the worker thread. The `if` | 549 | | - // condition is there so we only add the event handler once (if there are already pending | 550 | | - // async responses, the previous calls have added the event listener). | 551 | | - process.on('beforeExit', this.#beforeExitHandler); | | 539 | + // event loop of the worker thread is also empty), so we ref the worker until we get all the | | 540 | + // responses back. | | 541 | + this.#worker.ref(); | 552 | 542 | } | 553 | 543 |
| 554 | 544 | let response; | @@ -557,16 +547,13 @@ class HooksProxy { | 557 | 547 | await AtomicsWaitAsync(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, this.#workerNotificationLastId).value; | 558 | 548 | this.#workerNotificationLastId = AtomicsLoad(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); | 559 | 549 |
| 560 | | - // In case the beforeExit handler was called during the await, we revert its actions. | 561 | | - this.#worker.unref(); | 562 | | - | 563 | 550 | response = receiveMessageOnPort(asyncCommChannel.port1); | 564 | 551 | } while (response == null); | 565 | 552 | debug('got async response from worker', { method, args }, this.#lock); | 566 | 553 |
| 567 | 554 | if (--this.#numberOfPendingAsyncResponses === 0) { | 568 | 555 | // We got all the responses from the worker, its job is done (until next time). | 569 | | - process.off('beforeExit', this.#beforeExitHandler); | | 556 | + this.#worker.unref(); | 570 | 557 | } | 571 | 558 |
| 572 | 559 | const body = this.#unwrapMessage(response); | |
Collapse file Copy file name to clipboardExpand all lines: test/es-module/test-esm-loader-hooks.mjs +16Lines changed: 16 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -243,4 +243,20 @@ describe('Loader hooks', { concurrency: true }, () => { | 243 | 243 | assert.strictEqual(code, 42); | 244 | 244 | assert.strictEqual(signal, null); | 245 | 245 | }); | | 246 | + | | 247 | + it('should be fine to call `process.removeAllListeners("beforeExit")` from the main thread', async () => { | | 248 | + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ | | 249 | + '--no-warnings', | | 250 | + '--experimental-loader', | | 251 | + 'data:text/javascript,export function load(a,b,c){return new Promise(d=>setTimeout(()=>d(c(a,b)),99))}', | | 252 | + '--input-type=module', | | 253 | + '--eval', | | 254 | + 'setInterval(() => process.removeAllListeners("beforeExit"),1).unref();await import("data:text/javascript,")', | | 255 | + ]); | | 256 | + | | 257 | + assert.strictEqual(stderr, ''); | | 258 | + assert.strictEqual(stdout, ''); | | 259 | + assert.strictEqual(code, 0); | | 260 | + assert.strictEqual(signal, null); | | 261 | + }); | 246 | 262 | }); |
|