From ea7a63cd5aafcb20374688a2c7918fdc821ab113 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Mon, 4 Oct 2021 12:34:53 +0200 Subject: [PATCH] refactor(core): split opcall into sync/async (#12312) --- core/01_core.js | 8 ++--- core/bindings.rs | 83 ++++++++++++++++++++++++++++++++++++++---------- core/runtime.rs | 2 +- 3 files changed, 72 insertions(+), 21 deletions(-) diff --git a/core/01_core.js b/core/01_core.js index 44267e6fc2..5af3523408 100644 --- a/core/01_core.js +++ b/core/01_core.js @@ -23,7 +23,7 @@ } = window.__bootstrap.primordials; // Available on start due to bindings. - const { opcall } = window.Deno.core; + const { opcallSync, opcallAsync } = window.Deno.core; let opsCache = {}; const errorMap = {}; @@ -85,7 +85,7 @@ function syncOpsCache() { // op id 0 is a special value to retrieve the map of registered ops. - opsCache = ObjectFreeze(ObjectFromEntries(opcall(0))); + opsCache = ObjectFreeze(ObjectFromEntries(opcallSync(0))); } function opresolve() { @@ -125,14 +125,14 @@ function opAsync(opName, arg1 = null, arg2 = null) { const promiseId = nextPromiseId++; - const maybeError = opcall(opsCache[opName], promiseId, arg1, arg2); + const maybeError = opcallAsync(opsCache[opName], promiseId, arg1, arg2); // Handle sync error (e.g: error parsing args) if (maybeError) return unwrapOpResult(maybeError); return PromisePrototypeThen(setPromise(promiseId), unwrapOpResult); } function opSync(opName, arg1 = null, arg2 = null) { - return unwrapOpResult(opcall(opsCache[opName], null, arg1, arg2)); + return unwrapOpResult(opcallSync(opsCache[opName], arg1, arg2)); } function resources() { diff --git a/core/bindings.rs b/core/bindings.rs index aff6321619..7b3abc6024 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -7,6 +7,7 @@ use crate::JsRuntime; use crate::Op; use crate::OpId; use crate::OpPayload; +use crate::OpResult; use crate::OpTable; use crate::PromiseId; use crate::ZeroCopyBuf; @@ -31,7 +32,10 @@ lazy_static::lazy_static! { pub static ref EXTERNAL_REFERENCES: v8::ExternalReferences = v8::ExternalReferences::new(&[ v8::ExternalReference { - function: opcall.map_fn_to() + function: opcall_async.map_fn_to() + }, + v8::ExternalReference { + function: opcall_sync.map_fn_to() }, v8::ExternalReference { function: set_macrotask_callback.map_fn_to() @@ -135,7 +139,8 @@ pub fn initialize_context<'s>( deno_val.set(scope, core_key.into(), core_val.into()); // Bind functions to Deno.core.* - set_func(scope, core_val, "opcall", opcall); + set_func(scope, core_val, "opcallSync", opcall_sync); + set_func(scope, core_val, "opcallAsync", opcall_async); set_func( scope, core_val, @@ -303,13 +308,13 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { }; } -fn opcall<'s>( +fn opcall_sync<'s>( scope: &mut v8::HandleScope<'s>, args: v8::FunctionCallbackArguments, mut rv: v8::ReturnValue, ) { let state_rc = JsRuntime::state(scope); - let mut state = state_rc.borrow_mut(); + let state = state_rc.borrow(); let op_id = match v8::Local::::try_from(args.get(0)) .map(|l| l.value() as OpId) @@ -330,17 +335,59 @@ fn opcall<'s>( return; } + // Deserializable args (may be structured args or ZeroCopyBuf) + let a = args.get(1); + let b = args.get(2); + + let payload = OpPayload { + scope, + a, + b, + promise_id: 0, + }; + let op = OpTable::route_op(op_id, state.op_state.clone(), payload); + match op { + Op::Sync(result) => { + rv.set(result.to_v8(scope).unwrap()); + } + Op::NotFound => { + throw_type_error(scope, format!("Unknown op id: {}", op_id)); + } + // Async ops (ref or unref) + _ => { + throw_type_error( + scope, + format!("Can not call an async op [{}] with opSync()", op_id), + ); + } + } +} + +fn opcall_async<'s>( + scope: &mut v8::HandleScope<'s>, + args: v8::FunctionCallbackArguments, + mut rv: v8::ReturnValue, +) { + let state_rc = JsRuntime::state(scope); + let mut state = state_rc.borrow_mut(); + + let op_id = match v8::Local::::try_from(args.get(0)) + .map(|l| l.value() as OpId) + .map_err(AnyError::from) + { + Ok(op_id) => op_id, + Err(err) => { + throw_type_error(scope, format!("invalid op id: {}", err)); + return; + } + }; + // PromiseId let arg1 = args.get(1); - let promise_id = if arg1.is_null_or_undefined() { - Ok(0) // Accept null or undefined as 0 - } else { - // Otherwise expect int - v8::Local::::try_from(arg1) - .map(|l| l.value() as PromiseId) - .map_err(AnyError::from) - }; - // Fail if promise id invalid (not null/undefined or int) + let promise_id = v8::Local::::try_from(arg1) + .map(|l| l.value() as PromiseId) + .map_err(AnyError::from); + // Fail if promise id invalid (not an int) let promise_id: PromiseId = match promise_id { Ok(promise_id) => promise_id, Err(err) => { @@ -361,9 +408,13 @@ fn opcall<'s>( }; let op = OpTable::route_op(op_id, state.op_state.clone(), payload); match op { - Op::Sync(result) => { - rv.set(result.to_v8(scope).unwrap()); - } + Op::Sync(result) => match result { + OpResult::Ok(_) => throw_type_error( + scope, + format!("Can not call a sync op [{}] with opAsync()", op_id), + ), + OpResult::Err(_) => rv.set(result.to_v8(scope).unwrap()), + }, Op::Async(fut) => { state.pending_ops.push(fut); state.have_unpolled_ops = true; diff --git a/core/runtime.rs b/core/runtime.rs index 304ca68dbd..305052e9ae 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1861,7 +1861,7 @@ pub mod tests { r#" let thrown; try { - Deno.core.opcall(100, null, null, null); + Deno.core.opcallSync(100, null, null); } catch (e) { thrown = e; }