Skip to content

Commit

Permalink
fix: panic in worker when closing at top level (denoland#8510)
Browse files Browse the repository at this point in the history
Fixes panic occurring in worker when "self.close()" is called
at the top level, ie. worker shuts down while
module evaluation promise hasn't yet resolved.
  • Loading branch information
bartlomieju authored Nov 27, 2020
1 parent 28869a6 commit 22f951a
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 3 deletions.
1 change: 1 addition & 0 deletions cli/tests/immediately_close_worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
self.close();
5 changes: 5 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3273,6 +3273,11 @@ itest!(ignore_require {
exit_code: 0,
});

itest!(local_sources_not_cached_in_memory {
args: "run --allow-read --allow-write no_mem_cache.js",
output: "no_mem_cache.js.out",
});

#[test]
fn cafile_env_fetch() {
use deno_core::url::Url;
Expand Down
33 changes: 33 additions & 0 deletions cli/tests/no_mem_cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const fixtureFile = await Deno.makeTempFile();
let prefix = "file://";
if (Deno.build.os == "windows") {
prefix += "/";
}
const fixtureUrl = new URL(`${prefix}${fixtureFile}`);
let resolve;

let p = new Promise((res) => resolve = res);

await Deno.writeTextFile(fixtureUrl, `self.postMessage("hello");\n`);

const workerA = new Worker(fixtureUrl.href, { type: "module" });
workerA.onmessage = (msg) => {
console.log(msg.data);
resolve();
};

await p;
workerA.terminate();

p = new Promise((res) => resolve = res);

await Deno.writeTextFile(fixtureUrl, `self.postMessage("goodbye");\n`);

const workerB = new Worker(fixtureUrl.href, { type: "module" });
workerB.onmessage = (msg) => {
console.log(msg.data);
resolve();
};

await p;
workerB.terminate();
2 changes: 2 additions & 0 deletions cli/tests/no_mem_cache.js.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
hello
goodbye
16 changes: 16 additions & 0 deletions cli/tests/workers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,19 @@ Deno.test({
w.terminate();
},
});

Deno.test({
name: "Worker immediate close",
fn: async function (): Promise<void> {
const promise = deferred();
const w = new Worker(
new URL("./immediately_close_worker.js", import.meta.url).href,
{ type: "module" },
);
setTimeout(() => {
promise.resolve();
}, 1000);
await promise;
w.terminate();
},
});
10 changes: 7 additions & 3 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,9 +901,13 @@ impl JsRuntime {
let mut receiver = self.mod_evaluate_inner(id)?;

poll_fn(|cx| {
if let Poll::Ready(result) = receiver.poll_next_unpin(cx) {
debug!("received module evaluate");
return Poll::Ready(result.unwrap());
if let Poll::Ready(maybe_result) = receiver.poll_next_unpin(cx) {
debug!("received module evaluate {:#?}", maybe_result);
// If `None` is returned it means that runtime was destroyed before
// evaluation was complete. This can happen in Web Worker when `self.close()`
// is called at top level.
let result = maybe_result.unwrap_or(Ok(()));
return Poll::Ready(result);
}
let _r = self.poll_event_loop(cx)?;
Poll::Pending
Expand Down

0 comments on commit 22f951a

Please sign in to comment.