Skip to content

Commit

Permalink
chore(core): use oneshot channel in mod_evaluate() (denoland#11556)
Browse files Browse the repository at this point in the history
Oneshot is more appropriate because mod_evaluate() only sends a single
value.

It also makes it easier to use it correctly. As an embedder, I wasn't
sure if I'm expected to drain the channel or not.
  • Loading branch information
bnoordhuis committed Jul 30, 2021
1 parent 2b13bb6 commit c909faf
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 18 deletions.
10 changes: 5 additions & 5 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ mod tests {
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);
let _ = runtime.mod_evaluate(a_id);
futures::executor::block_on(runtime.run_event_loop(false)).unwrap();
let l = loads.lock();
assert_eq!(
Expand Down Expand Up @@ -1076,7 +1076,7 @@ mod tests {
runtime.instantiate_module(mod_a).unwrap();
assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);

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

Expand Down Expand Up @@ -1366,7 +1366,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);
let _ = runtime.mod_evaluate(circular1_id);
runtime.run_event_loop(false).await.unwrap();

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

runtime.mod_evaluate(main_id);
let _ = runtime.mod_evaluate(main_id);
futures::executor::block_on(runtime.run_event_loop(false)).unwrap();

let l = loads.lock();
Expand Down
15 changes: 7 additions & 8 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::OpPayload;
use crate::OpResult;
use crate::OpState;
use crate::PromiseId;
use futures::channel::mpsc;
use futures::channel::oneshot;
use futures::future::poll_fn;
use futures::future::FutureExt;
use futures::stream::FuturesUnordered;
Expand Down Expand Up @@ -96,7 +96,7 @@ struct DynImportModEvaluate {

struct ModEvaluate {
promise: v8::Global<v8::Promise>,
sender: mpsc::Sender<Result<(), AnyError>>,
sender: oneshot::Sender<Result<(), AnyError>>,
}

#[derive(Default, Clone)]
Expand Down Expand Up @@ -978,7 +978,7 @@ impl JsRuntime {
pub fn mod_evaluate(
&mut self,
id: ModuleId,
) -> mpsc::Receiver<Result<(), AnyError>> {
) -> oneshot::Receiver<Result<(), AnyError>> {
let state_rc = Self::state(self.v8_isolate());
let module_map_rc = Self::module_map(self.v8_isolate());
let scope = &mut self.handle_scope();
Expand All @@ -991,7 +991,7 @@ impl JsRuntime {
let mut status = module.get_status();
assert_eq!(status, v8::ModuleStatus::Instantiated);

let (sender, receiver) = mpsc::channel(1);
let (sender, receiver) = oneshot::channel();

// IMPORTANT: Top-level-await is enabled, which means that return value
// of module evaluation is a promise.
Expand Down Expand Up @@ -1238,7 +1238,6 @@ impl JsRuntime {
let scope = &mut self.handle_scope();

let promise = module_evaluation.promise.get(scope);
let mut sender = module_evaluation.sender.clone();
let promise_state = promise.state();

match promise_state {
Expand All @@ -1250,7 +1249,7 @@ impl JsRuntime {
v8::PromiseState::Fulfilled => {
scope.perform_microtask_checkpoint();
// Receiver end might have been already dropped, ignore the result
let _ = sender.try_send(Ok(()));
let _ = module_evaluation.sender.send(Ok(()));
}
v8::PromiseState::Rejected => {
let exception = promise.result(scope);
Expand All @@ -1259,7 +1258,7 @@ impl JsRuntime {
.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));
let _ = module_evaluation.sender.send(Err(err1));
}
}
}
Expand Down Expand Up @@ -1955,7 +1954,7 @@ pub mod tests {
)
.unwrap();

runtime.mod_evaluate(module_id);
let _ = runtime.mod_evaluate(module_id);
futures::executor::block_on(runtime.run_event_loop(false)).unwrap();

let _snapshot = runtime.snapshot();
Expand Down
4 changes: 2 additions & 2 deletions runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ impl WebWorker {

let mut receiver = self.js_runtime.mod_evaluate(id);
tokio::select! {
maybe_result = receiver.next() => {
maybe_result = &mut receiver => {
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()`
Expand All @@ -470,7 +470,7 @@ impl WebWorker {
return Ok(());
}
event_loop_result?;
let maybe_result = receiver.next().await;
let maybe_result = receiver.await;
maybe_result.unwrap_or(Ok(()))
}
}
Expand Down
5 changes: 2 additions & 3 deletions runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::ops;
use crate::permissions::Permissions;
use deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_core::error::AnyError;
use deno_core::futures::stream::StreamExt;
use deno_core::futures::Future;
use deno_core::located_script_name;
use deno_core::serde_json;
Expand Down Expand Up @@ -218,14 +217,14 @@ impl MainWorker {
self.wait_for_inspector_session();
let mut receiver = self.js_runtime.mod_evaluate(id);
tokio::select! {
maybe_result = receiver.next() => {
maybe_result = &mut receiver => {
debug!("received module evaluate {:#?}", maybe_result);
maybe_result.expect("Module evaluation result not provided.")
}

event_loop_result = self.run_event_loop(false) => {
event_loop_result?;
let maybe_result = receiver.next().await;
let maybe_result = receiver.await;
maybe_result.expect("Module evaluation result not provided.")
}
}
Expand Down

0 comments on commit c909faf

Please sign in to comment.