From 89bb774010b6b80bcbf7c19e8ed28f569abf4d90 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Thu, 22 Apr 2021 02:48:17 +0200 Subject: [PATCH] refactor(core): kill recv() and init() (#10299) `init()` was previously needed to init the shared queue, but now that it's gone `init()` only registers the async msg handler which is snapshot safe and constant since the op layer refactor. --- core/bindings.rs | 25 ------------------------- core/core.js | 9 ++------- core/runtime.rs | 36 ++++++++++++++++++------------------ 3 files changed, 20 insertions(+), 50 deletions(-) diff --git a/core/bindings.rs b/core/bindings.rs index 48450a6194..7467fe0a8e 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -25,9 +25,6 @@ lazy_static::lazy_static! { v8::ExternalReference { function: print.map_fn_to() }, - v8::ExternalReference { - function: recv.map_fn_to() - }, v8::ExternalReference { function: send.map_fn_to() }, @@ -122,7 +119,6 @@ pub fn initialize_context<'s>( // Bind functions to Deno.core.* set_func(scope, core_val, "print", print); - set_func(scope, core_val, "recv", recv); set_func(scope, core_val, "send", send); set_func( scope, @@ -321,27 +317,6 @@ fn print( } } -fn recv( - scope: &mut v8::HandleScope, - args: v8::FunctionCallbackArguments, - _rv: v8::ReturnValue, -) { - let state_rc = JsRuntime::state(scope); - let mut state = state_rc.borrow_mut(); - - let cb = match v8::Local::::try_from(args.get(0)) { - Ok(cb) => cb, - Err(err) => return throw_type_error(scope, err.to_string()), - }; - - let slot = match &mut state.js_recv_cb { - slot @ None => slot, - _ => return throw_type_error(scope, "Deno.core.recv() already called"), - }; - - slot.replace(v8::Global::new(scope, cb)); -} - fn send<'s>( scope: &mut v8::HandleScope<'s>, args: v8::FunctionCallbackArguments, diff --git a/core/core.js b/core/core.js index 0697458de3..a86ece4368 100644 --- a/core/core.js +++ b/core/core.js @@ -3,8 +3,7 @@ ((window) => { // Available on start due to bindings. - const core = window.Deno.core; - const { recv, send } = core; + const { send } = window.Deno.core; let opsCache = {}; const errorMap = {}; @@ -14,10 +13,6 @@ const NO_PROMISE = null; // Alias to null is faster than plain nulls const promiseRing = new Array(RING_SIZE).fill(NO_PROMISE); - function init() { - recv(handleAsyncMsgFromRust); - } - function setPromise(promiseId) { const idx = promiseId % RING_SIZE; // Move old promise from ring to map @@ -131,6 +126,6 @@ close, resources, registerErrorClass, - init, + handleAsyncMsgFromRust, }); })(this); diff --git a/core/runtime.rs b/core/runtime.rs index 1f87fef9af..ac33cce8d5 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -305,9 +305,8 @@ impl JsRuntime { if !has_startup_snapshot { js_runtime.js_init(); } - if !options.will_snapshot { - js_runtime.core_js_init(); + js_runtime.init_recv_cb(); } js_runtime @@ -352,14 +351,22 @@ impl JsRuntime { .unwrap(); } - /// Executes JavaScript code to initialize core.js, - /// specifically the js_recv_cb setter - /// - /// This function mustn't be called during snapshotting. - fn core_js_init(&mut self) { - self - .execute("deno:core/init.js", "Deno.core.init()") - .unwrap(); + /// Grabs a reference to core.js' handleAsyncMsgFromRust + fn init_recv_cb(&mut self) { + let context = self.global_context(); + let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context); + + // Get Deno.core.handleAsyncMsgFromRust + let code = + v8::String::new(scope, "Deno.core.handleAsyncMsgFromRust").unwrap(); + let script = v8::Script::compile(scope, code, None).unwrap(); + let v8_value = script.run(scope).unwrap(); + + // Put global handle in state.js_recv_cb + let state_rc = JsRuntime::state(scope); + let mut state = state_rc.borrow_mut(); + let cb = v8::Local::::try_from(v8_value).unwrap(); + state.js_recv_cb.replace(v8::Global::new(scope, cb)); } /// Returns the runtime's op state, which can be used to maintain ops @@ -1393,14 +1400,7 @@ impl JsRuntime { return Ok(()); } - // FIXME(bartlomieju): without check above this call would panic - // because of lazy initialization in core.js. It seems this lazy initialization - // hides unnecessary complexity. - let js_recv_cb_handle = state_rc - .borrow() - .js_recv_cb - .clone() - .expect("Deno.core.recv has not been called."); + let js_recv_cb_handle = state_rc.borrow().js_recv_cb.clone().unwrap(); let context = self.global_context(); let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);