From fc3a966a2d0be8fc76c384603bf18b55e0bbcf14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 13 Jun 2022 23:53:04 +0200 Subject: [PATCH] Deno.exit() is an alias to self.close() in worker contexts (#14826) This commit changes Deno.exit() to be an alias to self.close() in worker contexts, and the provided exit code becomes is ignored. --- cli/dts/lib.deno.ns.d.ts | 2 + cli/main.rs | 1 - cli/tests/integration/run_tests.rs | 6 +- .../op_exit_op_set_exit_code_in_worker.ts | 13 +++++ .../op_exit_op_set_exit_code_worker.js | 4 ++ cli/tests/testdata/set_exit_code_in_worker.ts | 13 ----- cli/tests/testdata/set_exit_code_worker.js | 4 -- runtime/js/99_main.js | 6 +- runtime/ops/os.rs | 56 +++++++++++++------ runtime/ops/worker_host.rs | 4 -- runtime/web_worker.rs | 4 +- 11 files changed, 66 insertions(+), 47 deletions(-) create mode 100644 cli/tests/testdata/op_exit_op_set_exit_code_in_worker.ts create mode 100644 cli/tests/testdata/op_exit_op_set_exit_code_worker.js delete mode 100644 cli/tests/testdata/set_exit_code_in_worker.ts delete mode 100644 cli/tests/testdata/set_exit_code_worker.js diff --git a/cli/dts/lib.deno.ns.d.ts b/cli/dts/lib.deno.ns.d.ts index f9c999cb3b..675f5e7e6c 100644 --- a/cli/dts/lib.deno.ns.d.ts +++ b/cli/dts/lib.deno.ns.d.ts @@ -480,6 +480,8 @@ declare namespace Deno { /** Exit the Deno process with optional exit code. If no exit code is supplied * then Deno will exit with return code of 0. * + * In worker contexts this is an alias to `self.close();`. + * * ```ts * Deno.exit(5); * ``` diff --git a/cli/main.rs b/cli/main.rs index 25f4728541..1f545d51f5 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -178,7 +178,6 @@ fn create_web_worker_callback( broadcast_channel: ps.broadcast_channel.clone(), shared_array_buffer_store: Some(ps.shared_array_buffer_store.clone()), compiled_wasm_module_store: Some(ps.compiled_wasm_module_store.clone()), - exit_code: args.exit_code, stdio: stdio.clone(), }; diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 7ea95c3123..aed1fe0fec 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -903,10 +903,10 @@ itest!(set_exit_code_2 { exit_code: 42, }); -itest!(set_exit_code_in_worker { - args: "run --no-check --unstable --allow-read set_exit_code_in_worker.ts", +itest!(op_exit_op_set_exit_code_in_worker { + args: "run --no-check --unstable --allow-read op_exit_op_set_exit_code_in_worker.ts", + exit_code: 21, output: "empty.out", - exit_code: 42, }); itest!(deno_exit_tampering { diff --git a/cli/tests/testdata/op_exit_op_set_exit_code_in_worker.ts b/cli/tests/testdata/op_exit_op_set_exit_code_in_worker.ts new file mode 100644 index 0000000000..74e8a53c52 --- /dev/null +++ b/cli/tests/testdata/op_exit_op_set_exit_code_in_worker.ts @@ -0,0 +1,13 @@ +// Set exit code to some value, we'll ensure that `Deno.exit()` and +// setting exit code in worker context is a no-op and is an alias for +// `self.close()`. + +// @ts-ignore Deno.core doesn't have type-defs +Deno.core.opSync("op_set_exit_code", 21); + +const worker = new Worker( + new URL("op_exit_op_set_exit_code_worker.js", import.meta.url).href, + { type: "module" }, +); + +worker.postMessage("go"); diff --git a/cli/tests/testdata/op_exit_op_set_exit_code_worker.js b/cli/tests/testdata/op_exit_op_set_exit_code_worker.js new file mode 100644 index 0000000000..54e5cd5629 --- /dev/null +++ b/cli/tests/testdata/op_exit_op_set_exit_code_worker.js @@ -0,0 +1,4 @@ +self.onmessage = () => { + Deno.core.opSync("op_set_exit_code", 42); + Deno.exit(); +}; diff --git a/cli/tests/testdata/set_exit_code_in_worker.ts b/cli/tests/testdata/set_exit_code_in_worker.ts deleted file mode 100644 index c49c6efb2f..0000000000 --- a/cli/tests/testdata/set_exit_code_in_worker.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { deferred } from "../../../test_util/std/async/deferred.ts"; - -const worker = new Worker( - new URL("set_exit_code_worker.js", import.meta.url).href, - { type: "module" }, -); - -const promise1 = deferred(); -worker.onmessage = (_e) => { - promise1.resolve(); -}; -await promise1; -worker.terminate(); diff --git a/cli/tests/testdata/set_exit_code_worker.js b/cli/tests/testdata/set_exit_code_worker.js deleted file mode 100644 index d1c0123d15..0000000000 --- a/cli/tests/testdata/set_exit_code_worker.js +++ /dev/null @@ -1,4 +0,0 @@ -// Set exit code -Deno.core.opSync("op_set_exit_code", 42); - -self.postMessage("ok"); diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index ae0de39371..e96482ba2b 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -581,7 +581,6 @@ delete Object.prototype.__proto__; defineEventHandler(window, "error"); defineEventHandler(window, "load"); defineEventHandler(window, "unload"); - const isUnloadDispatched = SymbolFor("isUnloadDispatched"); // Stores the flag for checking whether unload is dispatched or not. // This prevents the recursive dispatches of unload events. @@ -682,6 +681,11 @@ delete Object.prototype.__proto__; defineEventHandler(self, "message"); defineEventHandler(self, "error", undefined, true); + // `Deno.exit()` is an alias to `self.close()`. Setting and exit + // code using an op in worker context is a no-op. + os.setExitHandler((_exitCode) => { + workerClose(); + }); runtimeStart( runtimeOptions, diff --git a/runtime/ops/os.rs b/runtime/ops/os.rs index f138bae315..654bbede16 100644 --- a/runtime/ops/os.rs +++ b/runtime/ops/os.rs @@ -4,32 +4,36 @@ use super::utils::into_string; use crate::permissions::Permissions; use crate::worker::ExitCode; use deno_core::error::{type_error, AnyError}; -use deno_core::op; use deno_core::url::Url; use deno_core::Extension; use deno_core::OpState; +use deno_core::{op, ExtensionBuilder}; use serde::Serialize; use std::collections::HashMap; use std::env; +fn init_ops(builder: &mut ExtensionBuilder) -> &mut ExtensionBuilder { + builder.ops(vec![ + op_env::decl(), + op_exec_path::decl(), + op_exit::decl(), + op_delete_env::decl(), + op_get_env::decl(), + op_getgid::decl(), + op_getuid::decl(), + op_hostname::decl(), + op_loadavg::decl(), + op_network_interfaces::decl(), + op_os_release::decl(), + op_set_env::decl(), + op_set_exit_code::decl(), + op_system_memory_info::decl(), + ]) +} + pub fn init(exit_code: ExitCode) -> Extension { - Extension::builder() - .ops(vec![ - op_env::decl(), - op_exec_path::decl(), - op_exit::decl(), - op_delete_env::decl(), - op_get_env::decl(), - op_getgid::decl(), - op_getuid::decl(), - op_hostname::decl(), - op_loadavg::decl(), - op_network_interfaces::decl(), - op_os_release::decl(), - op_set_env::decl(), - op_set_exit_code::decl(), - op_system_memory_info::decl(), - ]) + let mut builder = Extension::builder(); + init_ops(&mut builder) .state(move |state| { state.put::(exit_code.clone()); Ok(()) @@ -37,6 +41,22 @@ pub fn init(exit_code: ExitCode) -> Extension { .build() } +pub fn init_for_worker() -> Extension { + let mut builder = Extension::builder(); + init_ops(&mut builder) + .middleware(|op| match op.name { + "op_exit" => noop_op::decl(), + "op_set_exit_code" => noop_op::decl(), + _ => op, + }) + .build() +} + +#[op] +fn noop_op(_code: i32) -> Result<(), AnyError> { + Ok(()) +} + #[op] fn op_exec_path(state: &mut OpState) -> Result { let current_exe = env::current_exe().unwrap(); diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs index 9860f7cb50..b61fca4608 100644 --- a/runtime/ops/worker_host.rs +++ b/runtime/ops/worker_host.rs @@ -11,7 +11,6 @@ use crate::web_worker::WebWorkerHandle; use crate::web_worker::WebWorkerType; use crate::web_worker::WorkerControlEvent; use crate::web_worker::WorkerId; -use crate::worker::ExitCode; use crate::worker::FormatJsErrorFn; use deno_core::error::AnyError; use deno_core::futures::future::LocalFutureObj; @@ -37,7 +36,6 @@ pub struct CreateWebWorkerArgs { pub permissions: Permissions, pub main_module: ModuleSpecifier, pub worker_type: WebWorkerType, - pub exit_code: ExitCode, } pub type CreateWebWorkerCb = dyn Fn(CreateWebWorkerArgs) -> (WebWorker, SendableWebWorkerHandle) @@ -171,7 +169,6 @@ fn op_create_worker( parent_permissions.clone() }; let parent_permissions = parent_permissions.clone(); - let exit_code = state.borrow::().clone(); let worker_id = state.take::(); let create_web_worker_cb = state.take::(); state.put::(create_web_worker_cb.clone()); @@ -207,7 +204,6 @@ fn op_create_worker( permissions: worker_permissions, main_module: module_specifier.clone(), worker_type, - exit_code, }); // Send thread safe handle from newly created worker to host thread diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index eb0aaf9165..ba2c016cc9 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -6,7 +6,6 @@ use crate::ops; use crate::ops::io::Stdio; use crate::permissions::Permissions; use crate::tokio_util::run_basic; -use crate::worker::ExitCode; use crate::worker::FormatJsErrorFn; use crate::BootstrapOptions; use deno_broadcast_channel::InMemoryBroadcastChannel; @@ -335,7 +334,6 @@ pub struct WebWorkerOptions { pub broadcast_channel: InMemoryBroadcastChannel, pub shared_array_buffer_store: Option, pub compiled_wasm_module_store: Option, - pub exit_code: ExitCode, pub stdio: Stdio, } @@ -421,7 +419,7 @@ impl WebWorker { unstable, options.unsafely_ignore_certificate_errors.clone(), ), - ops::os::init(options.exit_code), + ops::os::init_for_worker(), ops::permissions::init(), ops::process::init(), ops::spawn::init(),