summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSatya Rohith <me@satyarohith.com>2024-03-13 22:52:25 +0530
committerGitHub <noreply@github.com>2024-03-13 17:22:25 +0000
commit0fd8f549e2194223eca2d4b17f4e96cd5a0f5fd5 (patch)
tree76181b2a5f2991134f7343cfc6d4b8b755dbc333
parentb3ca3b2f25931afb350027bde87dc3d4f9a741b0 (diff)
fix(ext/node): allow automatic worker_thread termination (#22647)
Co-authored-by: Matt Mastracci <matthew@mastracci.com>
-rw-r--r--cli/worker.rs4
-rw-r--r--ext/node/polyfills/worker_threads.ts3
-rw-r--r--runtime/js/11_workers.js15
-rw-r--r--runtime/js/99_main.js13
-rw-r--r--runtime/ops/worker_host.rs3
-rw-r--r--runtime/web_worker.rs26
-rw-r--r--runtime/worker.rs2
-rw-r--r--runtime/worker_bootstrap.rs5
-rw-r--r--tests/integration/worker_tests.rs7
-rw-r--r--tests/testdata/workers/node_worker_auto_exits.mjs9
-rw-r--r--tests/testdata/workers/node_worker_auto_exits.mjs.out2
11 files changed, 64 insertions, 25 deletions
diff --git a/cli/worker.rs b/cli/worker.rs
index f0c7bfabc..85867a405 100644
--- a/cli/worker.rs
+++ b/cli/worker.rs
@@ -610,6 +610,7 @@ impl CliMainWorkerFactory {
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
verbose_deprecated_api_warning: shared.verbose_deprecated_api_warning,
future: shared.enable_future_features,
+ close_on_idle: true,
},
extensions: custom_extensions,
startup_snapshot: crate::js::deno_isolate_init(),
@@ -814,6 +815,7 @@ fn create_web_worker_callback(
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
verbose_deprecated_api_warning: shared.verbose_deprecated_api_warning,
future: false,
+ close_on_idle: args.close_on_idle,
},
extensions: vec![],
startup_snapshot: crate::js::deno_isolate_init(),
@@ -841,6 +843,8 @@ fn create_web_worker_callback(
stdio: stdio.clone(),
cache_storage_dir,
feature_checker,
+ strace_ops: shared.options.strace_ops.clone(),
+ close_on_idle: args.close_on_idle,
maybe_worker_metadata: args.maybe_worker_metadata,
};
diff --git a/ext/node/polyfills/worker_threads.ts b/ext/node/polyfills/worker_threads.ts
index 15b51aeb4..4563f157f 100644
--- a/ext/node/polyfills/worker_threads.ts
+++ b/ext/node/polyfills/worker_threads.ts
@@ -211,6 +211,7 @@ class NodeWorker extends EventEmitter {
permissions: null,
name: this.#name,
workerType: "module",
+ closeOnIdle: true,
},
serializedWorkerMetadata,
);
@@ -413,7 +414,7 @@ internals.__initWorkerThreads = (
>();
parentPort = self as ParentPort;
- if (typeof maybeWorkerMetadata !== "undefined") {
+ if (maybeWorkerMetadata) {
const { 0: metadata, 1: _ } = maybeWorkerMetadata;
workerData = metadata.workerData;
environmentData = metadata.environmentData;
diff --git a/runtime/js/11_workers.js b/runtime/js/11_workers.js
index 15bbad101..5d24df93d 100644
--- a/runtime/js/11_workers.js
+++ b/runtime/js/11_workers.js
@@ -46,6 +46,7 @@ function createWorker(
permissions,
name,
workerType,
+ closeOnIdle,
) {
return op_create_worker({
hasSourceCode,
@@ -54,6 +55,7 @@ function createWorker(
sourceCode,
specifier,
workerType,
+ closeOnIdle,
});
}
@@ -75,14 +77,6 @@ function hostRecvMessage(id) {
const privateWorkerRef = Symbol();
-function refWorker(worker) {
- worker[privateWorkerRef](true);
-}
-
-function unrefWorker(worker) {
- worker[privateWorkerRef](false);
-}
-
class Worker extends EventTarget {
#id = 0;
#name = "";
@@ -134,8 +128,9 @@ class Worker extends EventTarget {
hasSourceCode,
sourceCode,
deno?.permissions,
- name,
+ this.#name,
workerType,
+ false,
);
this.#id = id;
this.#pollControl();
@@ -325,4 +320,4 @@ webidl.converters["WorkerType"] = webidl.createEnumConverter("WorkerType", [
"module",
]);
-export { refWorker, unrefWorker, Worker };
+export { Worker };
diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js
index 27ba488e7..585128ba8 100644
--- a/runtime/js/99_main.js
+++ b/runtime/js/99_main.js
@@ -279,6 +279,7 @@ function postMessage(message, transferOrOptions = {}) {
let isClosing = false;
let globalDispatchEvent;
+let closeOnIdle;
async function pollForMessages() {
if (!globalDispatchEvent) {
@@ -288,7 +289,14 @@ async function pollForMessages() {
);
}
while (!isClosing) {
- const data = await op_worker_recv_message();
+ const op = op_worker_recv_message();
+ // In a Node.js worker, unref() the op promise to prevent it from
+ // keeping the event loop alive. This avoids the need to explicitly
+ // call self.close() or worker.terminate().
+ if (closeOnIdle) {
+ core.unrefOpPromise(op);
+ }
+ const data = await op;
if (data === null) break;
const v = messagePort.deserializeJsMessageData(data);
const message = v[0];
@@ -803,6 +811,8 @@ function bootstrapWorkerRuntime(
6: argv0,
7: shouldDisableDeprecatedApiWarning,
8: shouldUseVerboseDeprecatedApiWarning,
+ 9: _future,
+ 10: closeOnIdle_,
} = runtimeOptions;
deprecatedApiWarningDisabled = shouldDisableDeprecatedApiWarning;
@@ -864,6 +874,7 @@ function bootstrapWorkerRuntime(
location.setLocationHref(location_);
+ closeOnIdle = closeOnIdle_;
globalThis.pollForMessages = pollForMessages;
// TODO(bartlomieju): deprecate --unstable
diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs
index 1d056d459..3cfad5abb 100644
--- a/runtime/ops/worker_host.rs
+++ b/runtime/ops/worker_host.rs
@@ -35,6 +35,7 @@ pub struct CreateWebWorkerArgs {
pub permissions: PermissionsContainer,
pub main_module: ModuleSpecifier,
pub worker_type: WebWorkerType,
+ pub close_on_idle: bool,
pub maybe_worker_metadata: Option<JsMessageData>,
}
@@ -114,6 +115,7 @@ pub struct CreateWorkerArgs {
source_code: String,
specifier: String,
worker_type: WebWorkerType,
+ close_on_idle: bool,
}
/// Create worker as the host
@@ -191,6 +193,7 @@ fn op_create_worker(
permissions: worker_permissions,
main_module: module_specifier.clone(),
worker_type,
+ close_on_idle: args.close_on_idle,
maybe_worker_metadata,
});
diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs
index 07a8b374f..82da9de9e 100644
--- a/runtime/web_worker.rs
+++ b/runtime/web_worker.rs
@@ -5,6 +5,7 @@ use crate::permissions::PermissionsContainer;
use crate::shared::maybe_transpile_source;
use crate::shared::runtime;
use crate::tokio_util::create_and_run_current_thread;
+use crate::worker::create_op_metrics;
use crate::worker::import_meta_resolve_callback;
use crate::worker::validate_import_attributes_callback;
use crate::worker::FormatJsErrorFn;
@@ -34,7 +35,6 @@ use deno_core::ModuleCodeString;
use deno_core::ModuleId;
use deno_core::ModuleLoader;
use deno_core::ModuleSpecifier;
-use deno_core::OpMetricsSummaryTracker;
use deno_core::PollEventLoopOptions;
use deno_core::RuntimeOptions;
use deno_core::SharedArrayBufferStore;
@@ -327,6 +327,7 @@ pub struct WebWorker {
id: WorkerId,
pub js_runtime: JsRuntime,
pub name: String,
+ close_on_idle: bool,
internal_handle: WebWorkerInternalHandle,
pub worker_type: WebWorkerType,
pub main_module: ModuleSpecifier,
@@ -359,6 +360,8 @@ pub struct WebWorkerOptions {
pub cache_storage_dir: Option<std::path::PathBuf>,
pub stdio: Stdio,
pub feature_checker: Arc<FeatureChecker>,
+ pub strace_ops: Option<Vec<String>>,
+ pub close_on_idle: bool,
pub maybe_worker_metadata: Option<JsMessageData>,
}
@@ -511,17 +514,11 @@ impl WebWorker {
#[cfg(feature = "only_snapshotted_js_sources")]
options.startup_snapshot.as_ref().expect("A user snapshot was not provided, even though 'only_snapshotted_js_sources' is used.");
- // Hook up the summary metrics if the user or subcommand requested them
- let (op_summary_metrics, op_metrics_factory_fn) =
- if options.bootstrap.enable_op_summary_metrics {
- let op_summary_metrics = Rc::new(OpMetricsSummaryTracker::default());
- (
- Some(op_summary_metrics.clone()),
- Some(op_summary_metrics.op_metrics_factory_fn(|_| true)),
- )
- } else {
- (None, None)
- };
+ // Get our op metrics
+ let (op_summary_metrics, op_metrics_factory_fn) = create_op_metrics(
+ options.bootstrap.enable_op_summary_metrics,
+ options.strace_ops,
+ );
let mut js_runtime = JsRuntime::new(RuntimeOptions {
module_loader: Some(options.module_loader.clone()),
@@ -606,6 +603,7 @@ impl WebWorker {
main_module,
poll_for_messages_fn: None,
bootstrap_fn_global: Some(bootstrap_fn_global),
+ close_on_idle: options.close_on_idle,
maybe_worker_metadata: options.maybe_worker_metadata,
},
external_handle,
@@ -759,6 +757,10 @@ impl WebWorker {
return Poll::Ready(Err(e));
}
+ if self.close_on_idle {
+ return Poll::Ready(Ok(()));
+ }
+
// TODO(mmastrac): we don't want to test this w/classic workers because
// WPT triggers a failure here. This is only exposed via --enable-testing-features-do-not-use.
if self.worker_type == WebWorkerType::Module {
diff --git a/runtime/worker.rs b/runtime/worker.rs
index 2fd68dafe..97ea53980 100644
--- a/runtime/worker.rs
+++ b/runtime/worker.rs
@@ -227,7 +227,7 @@ impl Default for WorkerOptions {
}
}
-fn create_op_metrics(
+pub fn create_op_metrics(
enable_op_summary_metrics: bool,
strace_ops: Option<Vec<String>>,
) -> (
diff --git a/runtime/worker_bootstrap.rs b/runtime/worker_bootstrap.rs
index c019dae1a..e3e226b13 100644
--- a/runtime/worker_bootstrap.rs
+++ b/runtime/worker_bootstrap.rs
@@ -63,6 +63,7 @@ pub struct BootstrapOptions {
pub disable_deprecated_api_warning: bool,
pub verbose_deprecated_api_warning: bool,
pub future: bool,
+ pub close_on_idle: bool,
}
impl Default for BootstrapOptions {
@@ -94,6 +95,7 @@ impl Default for BootstrapOptions {
disable_deprecated_api_warning: false,
verbose_deprecated_api_warning: false,
future: false,
+ close_on_idle: false,
}
}
}
@@ -129,6 +131,8 @@ struct BootstrapV8<'a>(
bool,
// future
bool,
+ // close_on_idle
+ bool,
);
impl BootstrapOptions {
@@ -151,6 +155,7 @@ impl BootstrapOptions {
self.disable_deprecated_api_warning,
self.verbose_deprecated_api_warning,
self.future,
+ self.close_on_idle,
);
bootstrap.serialize(ser).unwrap()
diff --git a/tests/integration/worker_tests.rs b/tests/integration/worker_tests.rs
index 7b1bddead..492a06e36 100644
--- a/tests/integration/worker_tests.rs
+++ b/tests/integration/worker_tests.rs
@@ -111,3 +111,10 @@ itest!(worker_doest_stall_event_loop {
output: "workers/worker_doest_stall_event_loop.ts.out",
exit_code: 0,
});
+
+// Test for https://github.com/denoland/deno/issues/22629
+itest!(node_worker_auto_exits {
+ args: "run --quiet --allow-read workers/node_worker_auto_exits.mjs",
+ output: "workers/node_worker_auto_exits.mjs.out",
+ exit_code: 0,
+});
diff --git a/tests/testdata/workers/node_worker_auto_exits.mjs b/tests/testdata/workers/node_worker_auto_exits.mjs
new file mode 100644
index 000000000..abfb084c3
--- /dev/null
+++ b/tests/testdata/workers/node_worker_auto_exits.mjs
@@ -0,0 +1,9 @@
+import { isMainThread, Worker } from "node:worker_threads";
+
+if (isMainThread) {
+ // This re-loads the current file inside a Worker instance.
+ const w = new Worker(import.meta.filename);
+} else {
+ console.log("Inside Worker!");
+ console.log(isMainThread); // Prints 'false'.
+}
diff --git a/tests/testdata/workers/node_worker_auto_exits.mjs.out b/tests/testdata/workers/node_worker_auto_exits.mjs.out
new file mode 100644
index 000000000..18934d3ed
--- /dev/null
+++ b/tests/testdata/workers/node_worker_auto_exits.mjs.out
@@ -0,0 +1,2 @@
+Inside Worker!
+false