From 6e5f3453f806d6007b8f724837b5dd7d9eb17be9 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 20 Apr 2020 10:27:15 -0400 Subject: [PATCH] Remove core/plugin.rs (#4824) This simplifies the plugin interface in order to deliver op crates with a similar API --- cli/js/deno.ts | 2 +- cli/js/lib.deno.ns.d.ts | 34 +++++--------- cli/js/ops/plugins.ts | 12 ++--- cli/js/plugins.ts | 64 -------------------------- cli/ops/plugins.rs | 54 ++++------------------ cli/ops/resources.rs | 2 +- core/lib.rs | 2 - core/plugins.rs | 24 ---------- test_plugin/src/lib.rs | 13 +++--- test_plugin/tests/integration_tests.rs | 10 ++-- test_plugin/tests/test.js | 26 +++++++---- 11 files changed, 53 insertions(+), 190 deletions(-) delete mode 100644 cli/js/plugins.ts delete mode 100644 core/plugins.rs diff --git a/cli/js/deno.ts b/cli/js/deno.ts index 0322bac2782a46..e35e8c161e682e 100644 --- a/cli/js/deno.ts +++ b/cli/js/deno.ts @@ -93,7 +93,7 @@ export { PermissionStatus, Permissions, } from "./permissions.ts"; -export { openPlugin } from "./plugins.ts"; +export { openPlugin } from "./ops/plugins.ts"; export { kill } from "./ops/process.ts"; export { run, RunOptions, Process, ProcessStatus } from "./process.ts"; export { DirEntry, readdirSync, readdir } from "./ops/fs/read_dir.ts"; diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index 06baf3aa225000..119ba781e48b79 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -1796,35 +1796,23 @@ declare namespace Deno { * Requires `allow-write` permission. */ export function truncate(name: string, len?: number): Promise; - export interface AsyncHandler { - (msg: Uint8Array): void; - } - - export interface PluginOp { - dispatch( - control: Uint8Array, - zeroCopy?: ArrayBufferView | null - ): Uint8Array | null; - setAsyncHandler(handler: AsyncHandler): void; - } - - export interface Plugin { - ops: { - [name: string]: PluginOp; - }; - } - /** **UNSTABLE**: new API, yet to be vetted. * * Open and initalize a plugin. * - * const plugin = Deno.openPlugin("./path/to/some/plugin.so"); - * const some_op = plugin.ops.some_op; - * const response = some_op.dispatch(new Uint8Array([1,2,3,4])); + * const rid = Deno.openPlugin("./path/to/some/plugin.so"); + * const opId = Deno.core.ops()["some_op"]; + * const response = Deno.core.dispatch(opId, new Uint8Array([1,2,3,4])); * console.log(`Response from plugin ${response}`); * - * Requires `allow-plugin` permission. */ - export function openPlugin(filename: string): Plugin; + * Requires `allow-plugin` permission. + * + * The plugin system is not stable and will change in the future, hence the + * lack of docs. For now take a look at the example + * https://github.com/denoland/deno/tree/master/test_plugin + */ + export function openPlugin(filename: string): number; + export interface NetAddr { transport: "tcp" | "udp"; hostname: string; diff --git a/cli/js/ops/plugins.ts b/cli/js/ops/plugins.ts index 878ea1c66d8129..e4593afbb2cf2e 100644 --- a/cli/js/ops/plugins.ts +++ b/cli/js/ops/plugins.ts @@ -1,12 +1,6 @@ import { sendSync } from "./dispatch_json.ts"; -interface OpenPluginResponse { - rid: number; - ops: { - [name: string]: number; - }; -} - -export function openPlugin(filename: string): OpenPluginResponse { - return sendSync("op_open_plugin", { filename }); +export function openPlugin(filename: string): number { + const rid = sendSync("op_open_plugin", { filename }); + return rid; } diff --git a/cli/js/plugins.ts b/cli/js/plugins.ts deleted file mode 100644 index 498da8e16f23e4..00000000000000 --- a/cli/js/plugins.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { openPlugin as openPluginOp } from "./ops/plugins.ts"; -import { core } from "./core.ts"; -import { close } from "./ops/resources.ts"; - -export interface AsyncHandler { - (msg: Uint8Array): void; -} - -interface PluginOp { - dispatch( - control: Uint8Array, - zeroCopy?: ArrayBufferView | null - ): Uint8Array | null; - setAsyncHandler(handler: AsyncHandler): void; -} - -class PluginOpImpl implements PluginOp { - readonly #opId: number; - - constructor(opId: number) { - this.#opId = opId; - } - - dispatch( - control: Uint8Array, - zeroCopy?: ArrayBufferView | null - ): Uint8Array | null { - return core.dispatch(this.#opId, control, zeroCopy); - } - - setAsyncHandler(handler: AsyncHandler): void { - core.setAsyncHandler(this.#opId, handler); - } -} - -interface Plugin { - ops: { - [name: string]: PluginOp; - }; - close(): void; -} - -class PluginImpl implements Plugin { - #ops: { [name: string]: PluginOp } = {}; - - constructor(readonly rid: number, ops: { [name: string]: number }) { - for (const op in ops) { - this.#ops[op] = new PluginOpImpl(ops[op]); - } - } - - get ops(): { [name: string]: PluginOp } { - return Object.assign({}, this.#ops); - } - - close(): void { - close(this.rid); - } -} - -export function openPlugin(filename: string): Plugin { - const response = openPluginOp(filename); - return new PluginImpl(response.rid, response.ops); -} diff --git a/cli/ops/plugins.rs b/cli/ops/plugins.rs index c0dffc90f0bd65..8b905d786cb14f 100644 --- a/cli/ops/plugins.rs +++ b/cli/ops/plugins.rs @@ -4,16 +4,13 @@ use crate::op_error::OpError; use crate::ops::json_op; use crate::state::State; use deno_core::Isolate; -use deno_core::OpDispatcher; -use deno_core::OpId; -use deno_core::PluginInitContext; -use deno_core::PluginInitFn; use deno_core::ZeroCopyBuf; use dlopen::symbor::Library; -use std::collections::HashMap; use std::ffi::OsStr; use std::path::Path; +pub type PluginInitFn = fn(isolate: &mut deno_core::Isolate); + pub fn init(i: &mut Isolate, s: &State) { i.register_op( "op_open_plugin", @@ -23,27 +20,11 @@ pub fn init(i: &mut Isolate, s: &State) { fn open_plugin>(lib_path: P) -> Result { debug!("Loading Plugin: {:#?}", lib_path.as_ref()); - Library::open(lib_path).map_err(OpError::from) } struct PluginResource { lib: Library, - ops: HashMap, -} - -struct InitContext { - ops: HashMap>, -} - -impl PluginInitContext for InitContext { - fn register_op(&mut self, name: &str, op: Box) { - let existing = self.ops.insert(name.to_string(), op); - assert!( - existing.is_none(), - format!("Op already registered: {}", name) - ); - } } #[derive(Deserialize)] @@ -58,16 +39,13 @@ pub fn op_open_plugin( args: Value, _zero_copy: Option, ) -> Result { - let args: OpenPluginArgs = serde_json::from_value(args)?; + let args: OpenPluginArgs = serde_json::from_value(args).unwrap(); let filename = deno_fs::resolve_from_cwd(Path::new(&args.filename))?; state.check_plugin(&filename)?; - let lib = open_plugin(filename)?; - let plugin_resource = PluginResource { - lib, - ops: HashMap::new(), - }; + let lib = open_plugin(filename).unwrap(); + let plugin_resource = PluginResource { lib }; let mut state_ = state.borrow_mut(); let rid = state_ .resource_table @@ -77,27 +55,13 @@ pub fn op_open_plugin( .get_mut::(rid) .unwrap(); - let init_fn = *unsafe { + let deno_plugin_init = *unsafe { plugin_resource .lib .symbol::("deno_plugin_init") - }?; - let mut init_context = InitContext { - ops: HashMap::new(), - }; - init_fn(&mut init_context); - for op in init_context.ops { - // Register each plugin op in the `OpRegistry` with the name - // formated like this `plugin_{plugin_rid}_{name}`. - // The inclusion of prefix and rid is designed to avoid any - // op name collision beyond the bound of a single loaded - // plugin instance. - let op_id = isolate - .register_op(&format!("plugin_{}_{}", rid, op.0), state.core_op(op.1)); - plugin_resource.ops.insert(op.0, op_id); } + .unwrap(); + deno_plugin_init(isolate); - Ok(JsonOp::Sync( - json!({ "rid": rid, "ops": plugin_resource.ops }), - )) + Ok(JsonOp::Sync(json!(rid))) } diff --git a/cli/ops/resources.rs b/cli/ops/resources.rs index 4f49b2d9a70e37..3798318794bad5 100644 --- a/cli/ops/resources.rs +++ b/cli/ops/resources.rs @@ -29,7 +29,7 @@ fn op_close( struct CloseArgs { rid: i32, } - let args: CloseArgs = serde_json::from_value(args)?; + let args: CloseArgs = serde_json::from_value(args).unwrap(); let mut state = state.borrow_mut(); state .resource_table diff --git a/core/lib.rs b/core/lib.rs index d1776fb69846cc..661640910fe387 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -17,7 +17,6 @@ mod js_errors; mod module_specifier; mod modules; mod ops; -mod plugins; mod resources; mod shared_queue; @@ -31,7 +30,6 @@ pub use crate::js_errors::*; pub use crate::module_specifier::*; pub use crate::modules::*; pub use crate::ops::*; -pub use crate::plugins::*; pub use crate::resources::*; pub fn v8_version() -> &'static str { diff --git a/core/plugins.rs b/core/plugins.rs deleted file mode 100644 index a423790bca5561..00000000000000 --- a/core/plugins.rs +++ /dev/null @@ -1,24 +0,0 @@ -// TODO(ry) This plugin module is superfluous. Try to remove definitions for -// "init_fn!", "PluginInitFn", and "PluginInitContext". - -use crate::ops::OpDispatcher; - -pub type PluginInitFn = fn(context: &mut dyn PluginInitContext); - -pub trait PluginInitContext { - fn register_op( - &mut self, - name: &str, - op: Box, // TODO(ry) rename to dispatcher, not op. - ); -} - -#[macro_export] -macro_rules! init_fn { - ($fn:path) => { - #[no_mangle] - pub fn deno_plugin_init(context: &mut dyn PluginInitContext) { - $fn(context) - } - }; -} diff --git a/test_plugin/src/lib.rs b/test_plugin/src/lib.rs index c88052e9a0fb55..0c8655c5dd9327 100644 --- a/test_plugin/src/lib.rs +++ b/test_plugin/src/lib.rs @@ -1,17 +1,16 @@ -#[macro_use] extern crate deno_core; extern crate futures; +use deno_core::Buf; use deno_core::Op; -use deno_core::PluginInitContext; -use deno_core::{Buf, ZeroCopyBuf}; +use deno_core::ZeroCopyBuf; use futures::future::FutureExt; -fn init(context: &mut dyn PluginInitContext) { - context.register_op("testSync", Box::new(op_test_sync)); - context.register_op("testAsync", Box::new(op_test_async)); +#[no_mangle] +pub fn deno_plugin_init(isolate: &mut deno_core::Isolate) { + isolate.register_op("testSync", op_test_sync); + isolate.register_op("testAsync", op_test_async); } -init_fn!(init); pub fn op_test_sync( _isolate: &mut deno_core::Isolate, diff --git a/test_plugin/tests/integration_tests.rs b/test_plugin/tests/integration_tests.rs index 04a032635564a2..fc18d79d9ffd97 100644 --- a/test_plugin/tests/integration_tests.rs +++ b/test_plugin/tests/integration_tests.rs @@ -1,3 +1,7 @@ +// To run this test manually: +// cd test_plugin +// ../target/debug/deno --allow-plugin tests/test.js debug + // TODO(ry) Re-enable this test on windows. It is flaky for an unknown reason. #![cfg(not(windows))] @@ -38,11 +42,7 @@ fn basic() { println!("stderr {}", stderr); } assert!(output.status.success()); - let expected = if cfg!(target_os = "windows") { - "Hello from plugin. data: test | zero_copy: test\nPlugin Sync Response: test\r\nHello from plugin. data: test | zero_copy: test\nPlugin Async Response: test\r\n" - } else { - "Hello from plugin. data: test | zero_copy: test\nPlugin Sync Response: test\nHello from plugin. data: test | zero_copy: test\nPlugin Async Response: test\n" - }; + let expected = "Hello from plugin. data: test | zero_copy: test\nPlugin Sync Response: test\nHello from plugin. data: test | zero_copy: test\nPlugin Async Response: test\n"; assert_eq!(stdout, expected); assert_eq!(stderr, ""); } diff --git a/test_plugin/tests/test.js b/test_plugin/tests/test.js index d34624f099b07c..3661e107849d47 100644 --- a/test_plugin/tests/test.js +++ b/test_plugin/tests/test.js @@ -17,14 +17,21 @@ const filename = `../target/${Deno.args[0]}/${filenamePrefix}${filenameBase}${fi // in runTestClose() below. const resourcesPre = Deno.resources(); -const plugin = Deno.openPlugin(filename); +const rid = Deno.openPlugin(filename); -const { testSync, testAsync } = plugin.ops; +const { testSync, testAsync } = Deno.core.ops(); +if (!(testSync > 0)) { + throw "bad op id for testSync"; +} +if (!(testAsync > 0)) { + throw "bad op id for testAsync"; +} const textDecoder = new TextDecoder(); function runTestSync() { - const response = testSync.dispatch( + const response = Deno.core.dispatch( + testSync, new Uint8Array([116, 101, 115, 116]), new Uint8Array([116, 101, 115, 116]) ); @@ -32,12 +39,13 @@ function runTestSync() { console.log(`Plugin Sync Response: ${textDecoder.decode(response)}`); } -testAsync.setAsyncHandler((response) => { +Deno.core.setAsyncHandler(testAsync, (response) => { console.log(`Plugin Async Response: ${textDecoder.decode(response)}`); }); function runTestAsync() { - const response = testAsync.dispatch( + const response = Deno.core.dispatch( + testAsync, new Uint8Array([116, 101, 115, 116]), new Uint8Array([116, 101, 115, 116]) ); @@ -50,22 +58,22 @@ function runTestAsync() { function runTestOpCount() { const start = Deno.metrics(); - testSync.dispatch(new Uint8Array([116, 101, 115, 116])); + Deno.core.dispatch(testSync, new Uint8Array([116, 101, 115, 116])); const end = Deno.metrics(); - if (end.opsCompleted - start.opsCompleted !== 2) { + if (end.opsCompleted - start.opsCompleted !== 1) { // one op for the plugin and one for Deno.metrics throw new Error("The opsCompleted metric is not correct!"); } - if (end.opsDispatched - start.opsDispatched !== 2) { + if (end.opsDispatched - start.opsDispatched !== 1) { // one op for the plugin and one for Deno.metrics throw new Error("The opsDispatched metric is not correct!"); } } function runTestPluginClose() { - plugin.close(); + Deno.close(rid); const resourcesPost = Deno.resources();