summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--cli/tests/integration/run_tests.rs6
-rw-r--r--cli/tests/testdata/worker_drop_handle_race.js12
-rw-r--r--cli/tests/testdata/worker_drop_handle_race.js.out8
-rw-r--r--cli/tests/testdata/workers/drop_handle_race.js3
-rw-r--r--runtime/ops/worker_host.rs48
-rw-r--r--runtime/web_worker.rs6
6 files changed, 65 insertions, 18 deletions
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index df92ad422..5116db295 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -1188,6 +1188,12 @@ itest!(worker_close_race {
output: "worker_close_race.js.out",
});
+itest!(worker_drop_handle_race {
+ args: "run --quiet --reload --allow-read worker_drop_handle_race.js",
+ output: "worker_drop_handle_race.js.out",
+ exit_code: 1,
+});
+
itest!(worker_message_before_close {
args: "run --quiet --reload --allow-read worker_message_before_close.js",
output: "worker_message_before_close.js.out",
diff --git a/cli/tests/testdata/worker_drop_handle_race.js b/cli/tests/testdata/worker_drop_handle_race.js
new file mode 100644
index 000000000..d637ac8c2
--- /dev/null
+++ b/cli/tests/testdata/worker_drop_handle_race.js
@@ -0,0 +1,12 @@
+// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+
+// https://github.com/denoland/deno/issues/11342
+// Test for a panic that happens when the main thread's event loop finishes
+// running while the worker's event loop is still spinning.
+
+// The exception thrown in the worker will not terminate the worker, but it will
+// propagate to the main thread and cause it to exit.
+new Worker(
+ new URL("./workers/drop_handle_race.js", import.meta.url).href,
+ { type: "module" },
+);
diff --git a/cli/tests/testdata/worker_drop_handle_race.js.out b/cli/tests/testdata/worker_drop_handle_race.js.out
new file mode 100644
index 000000000..71dfde620
--- /dev/null
+++ b/cli/tests/testdata/worker_drop_handle_race.js.out
@@ -0,0 +1,8 @@
+error: Uncaught (in worker "") Error
+ throw new Error();
+ ^
+ at [WILDCARD]/workers/drop_handle_race.js:2:9
+ at fire (deno:ext/timers/[WILDCARD])
+ at handleTimerMacrotask (deno:ext/timers/[WILDCARD])
+error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+ at Worker.#pollControl (deno:runtime/js/11_workers.js:[WILDCARD])
diff --git a/cli/tests/testdata/workers/drop_handle_race.js b/cli/tests/testdata/workers/drop_handle_race.js
new file mode 100644
index 000000000..30676a600
--- /dev/null
+++ b/cli/tests/testdata/workers/drop_handle_race.js
@@ -0,0 +1,3 @@
+setTimeout(() => {
+ throw new Error();
+}, 1000);
diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs
index 829681ab6..f749e495c 100644
--- a/runtime/ops/worker_host.rs
+++ b/runtime/ops/worker_host.rs
@@ -65,7 +65,8 @@ pub type CreateWebWorkerCb = dyn Fn(CreateWebWorkerArgs) -> (WebWorker, Sendable
pub struct CreateWebWorkerCbHolder(Arc<CreateWebWorkerCb>);
pub struct WorkerThread {
- join_handle: JoinHandle<Result<(), AnyError>>,
+ // It's an Option so we can take the value before dropping the WorkerThread.
+ join_handle: Option<JoinHandle<Result<(), AnyError>>>,
worker_handle: WebWorkerHandle,
// A WorkerThread that hasn't been explicitly terminated can only be removed
@@ -75,6 +76,34 @@ pub struct WorkerThread {
message_closed: bool,
}
+impl WorkerThread {
+ fn terminate(mut self) {
+ self.worker_handle.clone().terminate();
+ self
+ .join_handle
+ .take()
+ .unwrap()
+ .join()
+ .expect("Worker thread panicked")
+ .expect("Panic in worker event loop");
+
+ // Optimization so the Drop impl doesn't try to terminate the worker handle
+ // again.
+ self.ctrl_closed = true;
+ self.message_closed = true;
+ }
+}
+
+impl Drop for WorkerThread {
+ fn drop(&mut self) {
+ // If either of the channels is closed, the worker thread has at least
+ // started closing, and its event loop won't start another run.
+ if !(self.ctrl_closed || self.message_closed) {
+ self.worker_handle.clone().terminate();
+ }
+ }
+}
+
pub type WorkersTable = HashMap<WorkerId, WorkerThread>;
pub fn init(create_web_worker_cb: Arc<CreateWebWorkerCb>) -> Extension {
@@ -557,7 +586,7 @@ fn op_create_worker(
let worker_handle = handle_receiver.recv().unwrap()?;
let worker_thread = WorkerThread {
- join_handle,
+ join_handle: Some(join_handle),
worker_handle: worker_handle.into(),
ctrl_closed: false,
message_closed: false,
@@ -578,12 +607,7 @@ fn op_host_terminate_worker(
_: (),
) -> Result<(), AnyError> {
if let Some(worker_thread) = state.borrow_mut::<WorkersTable>().remove(&id) {
- worker_thread.worker_handle.terminate();
- worker_thread
- .join_handle
- .join()
- .expect("Panic in worker thread")
- .expect("Panic in worker event loop");
+ worker_thread.terminate();
} else {
debug!("tried to terminate non-existent worker {}", id);
}
@@ -625,13 +649,7 @@ fn close_channel(
};
if terminate {
- let worker_thread = entry.remove();
- worker_thread.worker_handle.terminate();
- worker_thread
- .join_handle
- .join()
- .expect("Worker thread panicked")
- .expect("Panic in worker event loop");
+ entry.remove().terminate();
}
}
}
diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs
index 975028b16..07f6facc5 100644
--- a/runtime/web_worker.rs
+++ b/runtime/web_worker.rs
@@ -206,9 +206,9 @@ impl WebWorkerHandle {
/// Terminate the worker
/// This function will set terminated to true, terminate the isolate and close the message channel
pub fn terminate(self) {
- // This function can be called multiple times by whomever holds
- // the handle. However only a single "termination" should occur so
- // we need a guard here.
+ // A WebWorkerHandle can be terminated / dropped after `self.close()` has
+ // been called inside the worker, but only a single "termination" can occur,
+ // so we need a guard here.
let already_terminated = self.terminated.swap(true, Ordering::SeqCst);
if !already_terminated {