From a88f18723021a264bfaa5f9b10ac5bad573fd76f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 9 Jun 2022 16:05:31 +0200 Subject: [PATCH 1/4] wip --- node/process.ts | 8 +++++++- process.js | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 process.js diff --git a/node/process.ts b/node/process.ts index a4e35100f59a..b5c36cbfa882 100644 --- a/node/process.ts +++ b/node/process.ts @@ -49,7 +49,6 @@ import type { BindingName } from "./internal_binding/mod.ts"; import { buildAllowedFlags } from "./internal/process/per_thread.mjs"; const notImplementedEvents = [ - "beforeExit", "disconnect", "message", "multipleResolves", @@ -89,6 +88,9 @@ export const exit = (code?: number | string) => { if (!process._exiting) { process._exiting = true; + // FIXME(bartlomieju): this is wrong, we won't be using syscall to exit + // and thus the `unload` event will be emitted to properly trigget "emit" + // event on `process`. process.emit("exit", process.exitCode || 0); } @@ -266,6 +268,10 @@ class Process extends EventEmitter { constructor() { super(); + globalThis.addEventListener("beforeunload", () => { + super.emit("beforeExit", process.exitCode || 0); + }); + globalThis.addEventListener("unload", () => { if (!process._exiting) { process._exiting = true; diff --git a/process.js b/process.js new file mode 100644 index 000000000000..3e8bde400493 --- /dev/null +++ b/process.js @@ -0,0 +1,9 @@ +process.on('beforeExit', (code) => { + console.log('Process beforeExit event with code: ', code); +}); + +process.on('exit', (code) => { + console.log('Process exit event with code: ', code); +}); + +console.log('This message is displayed first.'); \ No newline at end of file From e3ba4586f11fbd19e687c9babda043d0043e084f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 21 Jun 2022 23:02:39 +0200 Subject: [PATCH 2/4] use Deno.core.eventLoopHasMoreWork() --- node/_core.ts | 3 +++ node/process.ts | 9 +++++++-- process.js | 10 +++++----- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/node/_core.ts b/node/_core.ts index d6fba4facb19..4b9e7d14a4ee 100644 --- a/node/_core.ts +++ b/node/_core.ts @@ -22,5 +22,8 @@ if (Deno?.core) { encode(chunk: string): Uint8Array { return new TextEncoder().encode(chunk); }, + eventLoopHasMoreWork(): boolean { + return false; + }, }; } diff --git a/node/process.ts b/node/process.ts index b5c36cbfa882..e1de77f41d70 100644 --- a/node/process.ts +++ b/node/process.ts @@ -36,6 +36,8 @@ import { stdin as stdin_, stdout as stdout_, } from "./_process/streams.mjs"; +import { core } from "./_core.ts"; + // TODO(kt3k): Give better types to stdio objects // deno-lint-ignore no-explicit-any const stderr = stderr_ as any; @@ -89,7 +91,7 @@ export const exit = (code?: number | string) => { if (!process._exiting) { process._exiting = true; // FIXME(bartlomieju): this is wrong, we won't be using syscall to exit - // and thus the `unload` event will be emitted to properly trigget "emit" + // and thus the `unload` event will not be emitted to properly trigger "emit" // event on `process`. process.emit("exit", process.exitCode || 0); } @@ -268,8 +270,11 @@ class Process extends EventEmitter { constructor() { super(); - globalThis.addEventListener("beforeunload", () => { + globalThis.addEventListener("beforeunload", (e) => { super.emit("beforeExit", process.exitCode || 0); + if (core.eventLoopHasMoreWork()) { + e.preventDefault(); + } }); globalThis.addEventListener("unload", () => { diff --git a/process.js b/process.js index 3e8bde400493..4f01fac76efc 100644 --- a/process.js +++ b/process.js @@ -1,9 +1,9 @@ -process.on('beforeExit', (code) => { - console.log('Process beforeExit event with code: ', code); +process.on("beforeExit", (code) => { + console.log("Process beforeExit event with code: ", code); }); -process.on('exit', (code) => { - console.log('Process exit event with code: ', code); +process.on("exit", (code) => { + console.log("Process exit event with code: ", code); }); -console.log('This message is displayed first.'); \ No newline at end of file +console.log("This message is displayed first."); From 6f8f3e7742e126d28144e22d420232a40b8e19ec Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 28 Jun 2022 16:23:25 -0400 Subject: [PATCH 3/4] add tests test-process-beforeexit.js is failing on line 77 --- node/_tools/config.json | 2 + .../test/parallel/test-process-beforeexit.js | 79 +++++++++++++++++++ .../test-process-exit-from-before-exit.js | 37 +++++++++ process.js | 9 --- 4 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 node/_tools/test/parallel/test-process-beforeexit.js create mode 100644 node/_tools/test/parallel/test-process-exit-from-before-exit.js delete mode 100644 process.js diff --git a/node/_tools/config.json b/node/_tools/config.json index 5f9130a47beb..df23b8bcd256 100644 --- a/node/_tools/config.json +++ b/node/_tools/config.json @@ -407,8 +407,10 @@ "test-path-win32-exists.js", "test-path-zero-length-strings.js", "test-path.js", + "test-process-beforeexit.js", "test-process-binding-internalbinding-allowlist.js", "test-process-env-allowed-flags.js", + "test-process-exit-from-before-exit.js", "test-process-exit-handler.js", "test-process-exit-recursive.js", "test-process-exit.js", diff --git a/node/_tools/test/parallel/test-process-beforeexit.js b/node/_tools/test/parallel/test-process-beforeexit.js new file mode 100644 index 000000000000..87cf40ac9f68 --- /dev/null +++ b/node/_tools/test/parallel/test-process-beforeexit.js @@ -0,0 +1,79 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 16.13.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const net = require('net'); + +process.once('beforeExit', common.mustCall(tryImmediate)); + +function tryImmediate() { + setImmediate(common.mustCall(() => { + process.once('beforeExit', common.mustCall(tryTimer)); + })); +} + +function tryTimer() { + setTimeout(common.mustCall(() => { + process.once('beforeExit', common.mustCall(tryListen)); + }), 1); +} + +function tryListen() { + net.createServer() + .listen(0) + .on('listening', common.mustCall(function() { + this.close(); + process.once('beforeExit', common.mustCall(tryRepeatedTimer)); + })); +} + +// Test that a function invoked from the beforeExit handler can use a timer +// to keep the event loop open, which can use another timer to keep the event +// loop open, etc. +// +// After N times, call function `tryNextTick` to test behaviors of the +// `process.nextTick`. +function tryRepeatedTimer() { + const N = 5; + let n = 0; + const repeatedTimer = common.mustCall(function() { + if (++n < N) + setTimeout(repeatedTimer, 1); + else // n == N + process.once('beforeExit', common.mustCall(tryNextTick)); + }, N); + setTimeout(repeatedTimer, 1); +} + +// Test if the callback of `process.nextTick` can be invoked. +function tryNextTick() { + process.nextTick(common.mustCall(function() { + process.once('beforeExit', common.mustNotCall()); + })); +} diff --git a/node/_tools/test/parallel/test-process-exit-from-before-exit.js b/node/_tools/test/parallel/test-process-exit-from-before-exit.js new file mode 100644 index 000000000000..5e6c53ce2f6c --- /dev/null +++ b/node/_tools/test/parallel/test-process-exit-from-before-exit.js @@ -0,0 +1,37 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 16.13.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +process.on('beforeExit', common.mustCall(function() { + setTimeout(common.mustNotCall(), 5); + process.exit(0); // Should execute immediately even if we schedule new work. + assert.fail(); +})); diff --git a/process.js b/process.js deleted file mode 100644 index 4f01fac76efc..000000000000 --- a/process.js +++ /dev/null @@ -1,9 +0,0 @@ -process.on("beforeExit", (code) => { - console.log("Process beforeExit event with code: ", code); -}); - -process.on("exit", (code) => { - console.log("Process exit event with code: ", code); -}); - -console.log("This message is displayed first."); From 55a466ec19fdac84694802788d298bc396d8e3fe Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 13 Jul 2022 14:52:09 -0400 Subject: [PATCH 4/4] suggestion from https://github.com/denoland/deno_std/pull/2331#discussion_r920234120 --- node/_next_tick.ts | 100 ++++++++++++++++++++++----------------------- node/process.ts | 2 + 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/node/_next_tick.ts b/node/_next_tick.ts index 7c193edc4bab..b8f6bba6e103 100644 --- a/node/_next_tick.ts +++ b/node/_next_tick.ts @@ -18,6 +18,56 @@ const queue = new FixedQueue(); // deno-lint-ignore no-explicit-any let _nextTick: any; +export function processTicksAndRejections() { + let tock; + do { + // deno-lint-ignore no-cond-assign + while (tock = queue.shift()) { + // FIXME(bartlomieju): Deno currently doesn't support async hooks + // const asyncId = tock[async_id_symbol]; + // emitBefore(asyncId, tock[trigger_async_id_symbol], tock); + + try { + const callback = (tock as Tock).callback; + if ((tock as Tock).args === undefined) { + callback(); + } else { + const args = (tock as Tock).args; + switch (args.length) { + case 1: + callback(args[0]); + break; + case 2: + callback(args[0], args[1]); + break; + case 3: + callback(args[0], args[1], args[2]); + break; + case 4: + callback(args[0], args[1], args[2], args[3]); + break; + default: + callback(...args); + } + } + } finally { + // FIXME(bartlomieju): Deno currently doesn't support async hooks + // if (destroyHooksExist()) + // emitDestroy(asyncId); + } + + // FIXME(bartlomieju): Deno currently doesn't support async hooks + // emitAfter(asyncId); + } + core.runMicrotasks(); + // FIXME(bartlomieju): Deno currently doesn't unhandled rejections + // } while (!queue.isEmpty() || processPromiseRejections()); + } while (!queue.isEmpty()); + core.setHasTickScheduled(false); + // FIXME(bartlomieju): Deno currently doesn't unhandled rejections + // setHasRejectionToWarn(false); +} + if (typeof core.setNextTickCallback !== "undefined") { function runNextTicks() { // FIXME(bartlomieju): Deno currently doesn't unhandled rejections @@ -36,56 +86,6 @@ if (typeof core.setNextTickCallback !== "undefined") { return true; } - function processTicksAndRejections() { - let tock; - do { - // deno-lint-ignore no-cond-assign - while (tock = queue.shift()) { - // FIXME(bartlomieju): Deno currently doesn't support async hooks - // const asyncId = tock[async_id_symbol]; - // emitBefore(asyncId, tock[trigger_async_id_symbol], tock); - - try { - const callback = (tock as Tock).callback; - if ((tock as Tock).args === undefined) { - callback(); - } else { - const args = (tock as Tock).args; - switch (args.length) { - case 1: - callback(args[0]); - break; - case 2: - callback(args[0], args[1]); - break; - case 3: - callback(args[0], args[1], args[2]); - break; - case 4: - callback(args[0], args[1], args[2], args[3]); - break; - default: - callback(...args); - } - } - } finally { - // FIXME(bartlomieju): Deno currently doesn't support async hooks - // if (destroyHooksExist()) - // emitDestroy(asyncId); - } - - // FIXME(bartlomieju): Deno currently doesn't support async hooks - // emitAfter(asyncId); - } - core.runMicrotasks(); - // FIXME(bartlomieju): Deno currently doesn't unhandled rejections - // } while (!queue.isEmpty() || processPromiseRejections()); - } while (!queue.isEmpty()); - core.setHasTickScheduled(false); - // FIXME(bartlomieju): Deno currently doesn't unhandled rejections - // setHasRejectionToWarn(false); - } - core.setNextTickCallback(processTicksAndRejections); core.setMacrotaskCallback(runNextTicks); diff --git a/node/process.ts b/node/process.ts index e1de77f41d70..d013fad03d6b 100644 --- a/node/process.ts +++ b/node/process.ts @@ -37,6 +37,7 @@ import { stdout as stdout_, } from "./_process/streams.mjs"; import { core } from "./_core.ts"; +import { processTicksAndRejections } from "./_next_tick.ts"; // TODO(kt3k): Give better types to stdio objects // deno-lint-ignore no-explicit-any @@ -272,6 +273,7 @@ class Process extends EventEmitter { globalThis.addEventListener("beforeunload", (e) => { super.emit("beforeExit", process.exitCode || 0); + processTicksAndRejections(); if (core.eventLoopHasMoreWork()) { e.preventDefault(); }