Skip to content

Commit

Permalink
Revert "fix: TLA in web worker (#8809)" (#8839)
Browse files Browse the repository at this point in the history
This reverts commit e924bbd.
  • Loading branch information
bartlomieju authored Dec 20, 2020
1 parent e924bbd commit 3eec73f
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 95 deletions.
15 changes: 0 additions & 15 deletions cli/tests/worker_with_top_level_await.ts

This file was deleted.

19 changes: 0 additions & 19 deletions cli/tests/workers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,22 +357,3 @@ Deno.test({
w.terminate();
},
});

Deno.test({
name: "Worker with top-level-await",
fn: async function (): Promise<void> {
const promise = deferred();
const worker = new Worker(
new URL("./worker_with_top_level_await.ts", import.meta.url).href,
{ deno: true, type: "module" },
);
worker.onmessage = (e): void => {
console.log("received from worker", e.data);
worker.postMessage("from main");
promise.resolve();
};

await promise;
worker.terminate();
},
});
14 changes: 6 additions & 8 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,8 +718,8 @@ mod tests {
let spec = ModuleSpecifier::resolve_url("file:///a.js").unwrap();
let a_id_fut = runtime.load_module(&spec, None);
let a_id = futures::executor::block_on(a_id_fut).expect("Failed to load");
runtime.mod_evaluate(a_id);
futures::executor::block_on(runtime.run_event_loop()).unwrap();

futures::executor::block_on(runtime.mod_evaluate(a_id)).unwrap();
let l = loads.lock().unwrap();
assert_eq!(
l.to_vec(),
Expand Down Expand Up @@ -786,8 +786,7 @@ mod tests {
let result = runtime.load_module(&spec, None).await;
assert!(result.is_ok());
let circular1_id = result.unwrap();
runtime.mod_evaluate(circular1_id);
runtime.run_event_loop().await.unwrap();
runtime.mod_evaluate(circular1_id).await.unwrap();

let l = loads.lock().unwrap();
assert_eq!(
Expand Down Expand Up @@ -864,8 +863,7 @@ mod tests {
println!(">> result {:?}", result);
assert!(result.is_ok());
let redirect1_id = result.unwrap();
runtime.mod_evaluate(redirect1_id);
runtime.run_event_loop().await.unwrap();
runtime.mod_evaluate(redirect1_id).await.unwrap();
let l = loads.lock().unwrap();
assert_eq!(
l.to_vec(),
Expand Down Expand Up @@ -1014,8 +1012,8 @@ mod tests {
.boxed_local();
let main_id =
futures::executor::block_on(main_id_fut).expect("Failed to load");
runtime.mod_evaluate(main_id);
futures::executor::block_on(runtime.run_event_loop()).unwrap();

futures::executor::block_on(runtime.mod_evaluate(main_id)).unwrap();

let l = loads.lock().unwrap();
assert_eq!(
Expand Down
36 changes: 23 additions & 13 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,17 +825,12 @@ impl JsRuntime {
Ok(())
}

// TODO(bartlomieju): make it return `ModuleEvaluationFuture`?
/// Evaluates an already instantiated ES module.
///
/// Returns a receiver handle that resolves when module promise resolves.
/// Implementors must manually call `run_event_loop()` to drive module
/// evaluation future.
///
/// `AnyError` can be downcast to a type that exposes additional information
/// about the V8 exception. By default this type is `JsError`, however it may
/// be a different type if `RuntimeOptions::js_error_create_fn` has been set.
pub fn mod_evaluate(
fn mod_evaluate_inner(
&mut self,
id: ModuleId,
) -> mpsc::Receiver<Result<(), AnyError>> {
Expand Down Expand Up @@ -907,6 +902,24 @@ impl JsRuntime {
receiver
}

pub async fn mod_evaluate(&mut self, id: ModuleId) -> Result<(), AnyError> {
let mut receiver = self.mod_evaluate_inner(id);

poll_fn(|cx| {
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
})
.await
}

fn dyn_import_error(&mut self, id: ModuleLoadId, err: AnyError) {
let state_rc = Self::state(self.v8_isolate());
let context = self.global_context();
Expand Down Expand Up @@ -1097,8 +1110,7 @@ impl JsRuntime {
v8::PromiseState::Fulfilled => {
state.pending_mod_evaluate.take();
scope.perform_microtask_checkpoint();
// Receiver end might have been already dropped, ignore the result
let _ = sender.try_send(Ok(()));
sender.try_send(Ok(())).unwrap();
}
v8::PromiseState::Rejected => {
let exception = promise.result(scope);
Expand All @@ -1108,8 +1120,7 @@ impl JsRuntime {
let err1 = exception_to_err_result::<()>(scope, exception, false)
.map_err(|err| attach_handle_to_error(scope, err, exception))
.unwrap_err();
// Receiver end might have been already dropped, ignore the result
let _ = sender.try_send(Err(err1));
sender.try_send(Err(err1)).unwrap();
}
}
}
Expand Down Expand Up @@ -2248,7 +2259,7 @@ pub mod tests {
runtime.mod_instantiate(mod_a).unwrap();
assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);

runtime.mod_evaluate(mod_a);
runtime.mod_evaluate_inner(mod_a);
assert_eq!(dispatch_count.load(Ordering::Relaxed), 1);
}

Expand Down Expand Up @@ -2491,8 +2502,7 @@ pub mod tests {
)
.unwrap();

runtime.mod_evaluate(module_id);
futures::executor::block_on(runtime.run_event_loop()).unwrap();
futures::executor::block_on(runtime.mod_evaluate(module_id)).unwrap();

let _snapshot = runtime.snapshot();
}
Expand Down
25 changes: 1 addition & 24 deletions runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,28 +315,7 @@ impl WebWorker {
module_specifier: &ModuleSpecifier,
) -> Result<(), AnyError> {
let id = self.js_runtime.load_module(module_specifier, None).await?;

let mut receiver = self.js_runtime.mod_evaluate(id);
tokio::select! {
maybe_result = receiver.next() => {
debug!("received worker 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 result;
}

event_loop_result = self.run_event_loop() => {
if self.has_been_terminated() {
return Ok(());
}
event_loop_result?;
let maybe_result = receiver.next().await;
let result = maybe_result.unwrap_or(Ok(()));
return result;
}
}
self.js_runtime.mod_evaluate(id).await
}

/// Returns a way to communicate with the Worker from other threads.
Expand Down Expand Up @@ -395,8 +374,6 @@ impl WebWorker {
let msg = String::from_utf8(msg.to_vec()).unwrap();
let script = format!("workerMessageRecvCallback({})", msg);

// TODO(bartlomieju): set proper script name like "deno:runtime/web_worker.js"
// so it's dimmed in stack trace instead of using "__anonymous__"
if let Err(e) = self.execute(&script) {
// If execution was terminated during message callback then
// just ignore it
Expand Down
17 changes: 1 addition & 16 deletions runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::permissions::Permissions;
use deno_core::error::AnyError;
use deno_core::futures::future::poll_fn;
use deno_core::futures::future::FutureExt;
use deno_core::futures::stream::StreamExt;
use deno_core::serde_json;
use deno_core::serde_json::json;
use deno_core::url::Url;
Expand Down Expand Up @@ -212,21 +211,7 @@ impl MainWorker {
) -> Result<(), AnyError> {
let id = self.preload_module(module_specifier).await?;
self.wait_for_inspector_session();
let mut receiver = self.js_runtime.mod_evaluate(id);
tokio::select! {
maybe_result = receiver.next() => {
debug!("received module evaluate {:#?}", maybe_result);
let result = maybe_result.expect("Module evaluation result not provided.");
return result;
}

event_loop_result = self.run_event_loop() => {
event_loop_result?;
let maybe_result = receiver.next().await;
let result = maybe_result.expect("Module evaluation result not provided.");
return result;
}
}
self.js_runtime.mod_evaluate(id).await
}

fn wait_for_inspector_session(&mut self) {
Expand Down

0 comments on commit 3eec73f

Please sign in to comment.