From bda937938550a0969588a6878d2fb6d72c17b22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 17 Sep 2020 18:09:50 +0200 Subject: [PATCH] refactor: move op_resources and op_close to deno_core (#7539) Moves op_close and op_resources to deno_core::ops and exports them. Adds serde dependency to deno_core and reexports it. Moves JS implementation of those ops to Deno.core and reexports them in Deno. --- cli/ops/mod.rs | 1 - cli/ops/resources.rs | 40 -------------------------------- cli/rt/11_resources.js | 23 ------------------ cli/rt/26_fetch.js | 11 ++++----- cli/rt/27_websocket.js | 5 ++-- cli/rt/30_files.js | 9 ++++--- cli/rt/30_net.js | 7 +++--- cli/rt/40_fs_events.js | 3 +-- cli/rt/40_process.js | 3 +-- cli/rt/40_repl.js | 3 +-- cli/rt/40_testing.js | 6 ++--- cli/rt/90_deno_ns.js | 2 -- cli/rt/99_main.js | 4 ++++ cli/tests/unit/resources_test.ts | 2 +- cli/web_worker.rs | 11 ++++++++- cli/worker.rs | 3 ++- core/core.js | 10 ++++++++ core/lib.rs | 2 ++ core/ops.rs | 35 ++++++++++++++++++++++++++++ core/resources.rs | 2 +- 20 files changed, 85 insertions(+), 97 deletions(-) delete mode 100644 cli/ops/resources.rs delete mode 100644 cli/rt/11_resources.js diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index 1e622463e3..288e3d0c02 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -19,7 +19,6 @@ pub mod plugin; pub mod process; pub mod random; pub mod repl; -pub mod resources; pub mod runtime; pub mod runtime_compiler; pub mod signal; diff --git a/cli/ops/resources.rs b/cli/ops/resources.rs deleted file mode 100644 index bd6ba29419..0000000000 --- a/cli/ops/resources.rs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. - -use deno_core::error::bad_resource_id; -use deno_core::error::AnyError; -use deno_core::OpState; -use deno_core::ZeroCopyBuf; -use serde::Deserialize; -use serde_json::Value; - -pub fn init(rt: &mut deno_core::JsRuntime) { - super::reg_json_sync(rt, "op_resources", op_resources); - super::reg_json_sync(rt, "op_close", op_close); -} - -fn op_resources( - state: &mut OpState, - _args: Value, - _zero_copy: &mut [ZeroCopyBuf], -) -> Result { - let serialized_resources = state.resource_table.entries(); - Ok(json!(serialized_resources)) -} - -/// op_close removes a resource from the resource table. -fn op_close( - state: &mut OpState, - args: Value, - _zero_copy: &mut [ZeroCopyBuf], -) -> Result { - #[derive(Deserialize)] - struct CloseArgs { - rid: i32, - } - let args: CloseArgs = serde_json::from_value(args)?; - state - .resource_table - .close(args.rid as u32) - .ok_or_else(bad_resource_id)?; - Ok(json!({})) -} diff --git a/cli/rt/11_resources.js b/cli/rt/11_resources.js deleted file mode 100644 index 9a1595b78c..0000000000 --- a/cli/rt/11_resources.js +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. - -((window) => { - const core = window.Deno.core; - - function resources() { - const res = core.jsonOpSync("op_resources"); - const resources = {}; - for (const resourceTuple of res) { - resources[resourceTuple[0]] = resourceTuple[1]; - } - return resources; - } - - function close(rid) { - core.jsonOpSync("op_close", { rid }); - } - - window.__bootstrap.resources = { - close, - resources, - }; -})(this); diff --git a/cli/rt/26_fetch.js b/cli/rt/26_fetch.js index ca36d7ba43..b337fbe109 100644 --- a/cli/rt/26_fetch.js +++ b/cli/rt/26_fetch.js @@ -5,7 +5,6 @@ const { notImplemented } = window.__bootstrap.util; const { getHeaderValueParams, isTypedArray } = window.__bootstrap.webUtil; const { Blob, bytesSymbol: blobBytesSymbol } = window.__bootstrap.blob; - const { close } = window.__bootstrap.resources; const Body = window.__bootstrap.body; const { ReadableStream } = window.__bootstrap.streams; const { MultipartBuilder } = window.__bootstrap.multipart; @@ -24,7 +23,7 @@ this.rid = rid; } close() { - close(this.rid); + core.close(this.rid); } } @@ -290,7 +289,7 @@ ) { // We won't use body of received response, so close it now // otherwise it will be kept in resource table. - close(fetchResponse.bodyRid); + core.close(fetchResponse.bodyRid); responseBody = null; } else { responseBody = new ReadableStream({ @@ -300,7 +299,7 @@ const result = await core.jsonOpAsync("op_fetch_read", { rid }); if (!result || !result.chunk) { controller.close(); - close(rid); + core.close(rid); } else { // TODO(ry) This is terribly inefficient. Make this zero-copy. const chunk = new Uint8Array(result.chunk); @@ -309,12 +308,12 @@ } catch (e) { controller.error(e); controller.close(); - close(rid); + core.close(rid); } }, cancel() { // When reader.cancel() is called - close(rid); + core.close(rid); }, }); } diff --git a/cli/rt/27_websocket.js b/cli/rt/27_websocket.js index d7fc031690..ee114170d8 100644 --- a/cli/rt/27_websocket.js +++ b/cli/rt/27_websocket.js @@ -2,7 +2,6 @@ ((window) => { const core = window.Deno.core; - const { close } = window.__bootstrap.resources; const { requiredArguments } = window.__bootstrap.webUtil; const CONNECTING = 0; const OPEN = 1; @@ -71,7 +70,7 @@ event.target = this; this.onclose?.(event); this.dispatchEvent(event); - close(this.#rid); + core.close(this.#rid); }); const event = new Event("error"); @@ -242,7 +241,7 @@ event.target = this; this.onclose?.(event); this.dispatchEvent(event); - close(this.#rid); + core.close(this.#rid); }); } } diff --git a/cli/rt/30_files.js b/cli/rt/30_files.js index ff6b85ae53..e492da2188 100644 --- a/cli/rt/30_files.js +++ b/cli/rt/30_files.js @@ -2,7 +2,6 @@ ((window) => { const core = window.Deno.core; - const { close } = window.__bootstrap.resources; const { read, readSync, write, writeSync } = window.__bootstrap.io; const { pathFromURL } = window.__bootstrap.util; @@ -104,7 +103,7 @@ } close() { - close(this.rid); + core.close(this.rid); } } @@ -122,7 +121,7 @@ } close() { - close(this.rid); + core.close(this.rid); } } @@ -140,7 +139,7 @@ } close() { - close(this.rid); + core.close(this.rid); } } @@ -158,7 +157,7 @@ } close() { - close(this.rid); + core.close(this.rid); } } diff --git a/cli/rt/30_net.js b/cli/rt/30_net.js index 8c3dcb4ba6..53de5200d2 100644 --- a/cli/rt/30_net.js +++ b/cli/rt/30_net.js @@ -4,7 +4,6 @@ const core = window.Deno.core; const { errors } = window.__bootstrap.errors; const { read, write } = window.__bootstrap.io; - const { close } = window.__bootstrap.resources; const ShutdownMode = { // See http://man7.org/linux/man-pages/man2/shutdown.2.html @@ -88,7 +87,7 @@ } close() { - close(this.rid); + core.close(this.rid); } // TODO(lucacasonato): make this unavailable in stable @@ -138,7 +137,7 @@ } close() { - close(this.rid); + core.close(this.rid); } [Symbol.asyncIterator]() { @@ -187,7 +186,7 @@ } close() { - close(this.rid); + core.close(this.rid); } async *[Symbol.asyncIterator]() { diff --git a/cli/rt/40_fs_events.js b/cli/rt/40_fs_events.js index 43c0a4b92d..a36adecba5 100644 --- a/cli/rt/40_fs_events.js +++ b/cli/rt/40_fs_events.js @@ -3,7 +3,6 @@ ((window) => { const core = window.Deno.core; const { errors } = window.__bootstrap.errors; - const { close } = window.__bootstrap.resources; class FsWatcher { #rid = 0; @@ -31,7 +30,7 @@ } return(value) { - close(this.rid); + core.close(this.rid); return Promise.resolve({ value, done: true }); } diff --git a/cli/rt/40_process.js b/cli/rt/40_process.js index e188e9c6ae..b46a1aead5 100644 --- a/cli/rt/40_process.js +++ b/cli/rt/40_process.js @@ -3,7 +3,6 @@ ((window) => { const core = window.Deno.core; const { File } = window.__bootstrap.files; - const { close } = window.__bootstrap.resources; const { readAll } = window.__bootstrap.buffer; const { assert, pathFromURL } = window.__bootstrap.util; @@ -78,7 +77,7 @@ } close() { - close(this.rid); + core.close(this.rid); } kill(signo) { diff --git a/cli/rt/40_repl.js b/cli/rt/40_repl.js index 8b05aa4c3d..273687d930 100644 --- a/cli/rt/40_repl.js +++ b/cli/rt/40_repl.js @@ -4,7 +4,6 @@ const core = window.Deno.core; const exit = window.__bootstrap.os.exit; const version = window.__bootstrap.version.version; - const close = window.__bootstrap.resources.close; const inspectArgs = window.__bootstrap.console.inspectArgs; function opStartRepl(historyFile) { @@ -90,7 +89,7 @@ const quitRepl = (exitCode) => { // Special handling in case user calls deno.close(3). try { - close(rid); // close signals Drop on REPL and saves history. + core.close(rid); // close signals Drop on REPL and saves history. } catch {} exit(exitCode); }; diff --git a/cli/rt/40_testing.js b/cli/rt/40_testing.js index 128f8ca931..c374e0ca62 100644 --- a/cli/rt/40_testing.js +++ b/cli/rt/40_testing.js @@ -1,13 +1,13 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. ((window) => { + const core = window.Deno.core; const { gray, green, italic, red, yellow } = window.__bootstrap.colors; const { exit } = window.__bootstrap.os; const { Console, inspectArgs } = window.__bootstrap.console; const { stdout } = window.__bootstrap.files; const { exposeForTest } = window.__bootstrap.internals; const { metrics } = window.__bootstrap.metrics; - const { resources } = window.__bootstrap.resources; const { assert } = window.__bootstrap.util; const disabledConsole = new Console(() => {}); @@ -64,9 +64,9 @@ finishing test case.`, fn, ) { return async function resourceSanitizer() { - const pre = resources(); + const pre = core.resources(); await fn(); - const post = resources(); + const post = core.resources(); const preStr = JSON.stringify(pre, null, 2); const postStr = JSON.stringify(post, null, 2); diff --git a/cli/rt/90_deno_ns.js b/cli/rt/90_deno_ns.js index 23ab12924e..3a41e6bd11 100644 --- a/cli/rt/90_deno_ns.js +++ b/cli/rt/90_deno_ns.js @@ -53,8 +53,6 @@ __bootstrap.denoNs = { env: __bootstrap.os.env, exit: __bootstrap.os.exit, execPath: __bootstrap.os.execPath, - resources: __bootstrap.resources.resources, - close: __bootstrap.resources.close, Buffer: __bootstrap.buffer.Buffer, readAll: __bootstrap.buffer.readAll, readAllSync: __bootstrap.buffer.readAllSync, diff --git a/cli/rt/99_main.js b/cli/rt/99_main.js index 03bd77be49..02834dcf34 100644 --- a/cli/rt/99_main.js +++ b/cli/rt/99_main.js @@ -308,6 +308,8 @@ delete Object.prototype.__proto__; core, internal: internalSymbol, [internalSymbol]: internalObject, + resources: core.resources, + close: core.close, ...denoNs, }; Object.defineProperties(finalDenoNs, { @@ -361,6 +363,8 @@ delete Object.prototype.__proto__; core, internal: internalSymbol, [internalSymbol]: internalObject, + resources: core.resources, + close: core.close, ...denoNs, }; if (useDenoNamespace) { diff --git a/cli/tests/unit/resources_test.ts b/cli/tests/unit/resources_test.ts index 5742fd6a05..4e4288ff9f 100644 --- a/cli/tests/unit/resources_test.ts +++ b/cli/tests/unit/resources_test.ts @@ -4,7 +4,7 @@ import { unitTest, assertEquals, assert, assertThrows } from "./test_util.ts"; unitTest(function resourcesCloseBadArgs(): void { assertThrows(() => { Deno.close((null as unknown) as number); - }, Deno.errors.InvalidData); + }, TypeError); }); unitTest(function resourcesStdio(): void { diff --git a/cli/web_worker.rs b/cli/web_worker.rs index 3484a598d1..bad017914a 100644 --- a/cli/web_worker.rs +++ b/cli/web_worker.rs @@ -117,7 +117,16 @@ impl WebWorker { ops::worker_host::init(&mut web_worker.worker); ops::idna::init(&mut web_worker.worker); ops::io::init(&mut web_worker.worker); - ops::resources::init(&mut web_worker.worker); + ops::reg_json_sync( + &mut web_worker.worker, + "op_close", + deno_core::op_close, + ); + ops::reg_json_sync( + &mut web_worker.worker, + "op_resources", + deno_core::op_resources, + ); ops::errors::init(&mut web_worker.worker); ops::timers::init(&mut web_worker.worker); ops::fetch::init(&mut web_worker.worker); diff --git a/cli/worker.rs b/cli/worker.rs index 6d5f37175d..6c22ba92bf 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -287,7 +287,8 @@ impl MainWorker { ops::process::init(&mut worker); ops::random::init(&mut worker); ops::repl::init(&mut worker); - ops::resources::init(&mut worker); + ops::reg_json_sync(&mut worker, "op_close", deno_core::op_close); + ops::reg_json_sync(&mut worker, "op_resources", deno_core::op_resources); ops::signal::init(&mut worker); ops::timers::init(&mut worker); ops::tty::init(&mut worker); diff --git a/core/core.js b/core/core.js index b0de55b2c7..7b4e24702b 100644 --- a/core/core.js +++ b/core/core.js @@ -256,6 +256,14 @@ SharedQueue Binary Layout promise.resolve(res); } + function resources() { + return jsonOpSync("op_resources"); + } + + function close(rid) { + jsonOpSync("op_close", { rid }); + } + Object.assign(window.Deno.core, { jsonOpAsync, jsonOpSync, @@ -263,6 +271,8 @@ SharedQueue Binary Layout dispatch: send, dispatchByName: dispatch, ops, + close, + resources, registerErrorClass, getErrorClass, // sharedQueue is private but exposed for testing. diff --git a/core/lib.rs b/core/lib.rs index c0302ff83d..c17399aab1 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -37,6 +37,8 @@ pub use crate::modules::RecursiveModuleLoad; pub use crate::normalize_path::normalize_path; pub use crate::ops::json_op_async; pub use crate::ops::json_op_sync; +pub use crate::ops::op_close; +pub use crate::ops::op_resources; pub use crate::ops::Op; pub use crate::ops::OpAsyncFuture; pub use crate::ops::OpFn; diff --git a/core/ops.rs b/core/ops.rs index 95be85168a..b7507156cd 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -1,5 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. +use crate::error::bad_resource_id; use crate::error::type_error; use crate::error::AnyError; use crate::gotham_state::GothamState; @@ -7,6 +8,7 @@ use crate::BufVec; use crate::ZeroCopyBuf; use futures::Future; use indexmap::IndexMap; +use serde_json::json; use serde_json::Value; use std::cell::RefCell; use std::collections::HashMap; @@ -217,3 +219,36 @@ fn json_serialize_op_result( }; serde_json::to_vec(&value).unwrap().into_boxed_slice() } + +/// Return map of resources with id as key +/// and string representaion as value. +/// +/// This op must be wrapped in `json_op_sync`. +pub fn op_resources( + state: &mut OpState, + _args: Value, + _zero_copy: &mut [ZeroCopyBuf], +) -> Result { + let serialized_resources = state.resource_table.entries(); + Ok(json!(serialized_resources)) +} + +/// Remove a resource from the resource table. +/// +/// This op must be wrapped in `json_op_sync`. +pub fn op_close( + state: &mut OpState, + args: Value, + _zero_copy: &mut [ZeroCopyBuf], +) -> Result { + let rid = args + .get("rid") + .and_then(Value::as_u64) + .ok_or_else(|| type_error("missing or invalid `rid`"))?; + + state + .resource_table + .close(rid as u32) + .ok_or_else(bad_resource_id)?; + Ok(json!({})) +} diff --git a/core/resources.rs b/core/resources.rs index 51d3260dc3..0a159425ca 100644 --- a/core/resources.rs +++ b/core/resources.rs @@ -52,7 +52,7 @@ impl ResourceTable { rid } - pub fn entries(&self) -> Vec<(ResourceId, String)> { + pub fn entries(&self) -> HashMap { self .map .iter()