Skip to content

Commit

Permalink
feat(core): disableable extensions & ops (denoland#14063)
Browse files Browse the repository at this point in the history
Streamlines a common middleware pattern and provides foundations for avoiding variably sized v8::ExternalReferences & enabling fully monomorphic op callpaths
  • Loading branch information
AaronO committed Mar 22, 2022
1 parent c9817c3 commit f81334d
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 42 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ jobs:
~/.cargo/registry/index
~/.cargo/registry/cache
~/.cargo/git/db
key: 1-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}
key: 2-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}

# In main branch, always creates fresh cache
- name: Cache build output (main)
Expand All @@ -252,7 +252,7 @@ jobs:
!./target/*/*.zip
!./target/*/*.tar.gz
key: |
1-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ github.sha }}
2-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ github.sha }}
# Restore cache from the latest 'main' branch build.
- name: Cache build output (PR)
Expand All @@ -268,7 +268,7 @@ jobs:
!./target/*/*.tar.gz
key: never_saved
restore-keys: |
1-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-
2-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-
# Don't save cache after building PRs or branches other than 'main'.
- name: Skip save cache (PR)
Expand Down
2 changes: 1 addition & 1 deletion core/examples/disable_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use deno_core::RuntimeOptions;
fn main() {
let my_ext = Extension::builder()
.middleware(|op| match op.name {
"op_print" => deno_core::op_void_sync::decl(),
"op_print" => op.disable(),
_ => op,
})
.build();
Expand Down
28 changes: 27 additions & 1 deletion core/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ pub type OpEventLoopFn = dyn Fn(&mut OpState, &mut Context) -> bool;
pub struct OpDecl {
pub name: &'static str,
pub v8_fn_ptr: OpFnRef,
pub enabled: bool,
pub is_async: bool, // TODO(@AaronO): enum sync/async/fast ?
}

impl OpDecl {
pub fn enabled(self, enabled: bool) -> Self {
Self { enabled, ..self }
}

pub fn disable(self) -> Self {
self.enabled(false)
}
}

#[derive(Default)]
Expand All @@ -24,6 +36,7 @@ pub struct Extension {
middleware_fn: Option<Box<OpMiddlewareFn>>,
event_loop_middleware: Option<Box<OpEventLoopFn>>,
initialized: bool,
enabled: bool,
}

// Note: this used to be a trait, but we "downgraded" it to a single concrete type
Expand All @@ -50,7 +63,11 @@ impl Extension {
}
self.initialized = true;

self.ops.take()
let mut ops = self.ops.take()?;
for op in ops.iter_mut() {
op.enabled = self.enabled && op.enabled;
}
Some(ops)
}

/// Allows setting up the initial op-state of an isolate at startup.
Expand Down Expand Up @@ -81,6 +98,14 @@ impl Extension {
.map(|f| f(op_state, cx))
.unwrap_or(false)
}

pub fn enabled(self, enabled: bool) -> Self {
Self { enabled, ..self }
}

pub fn disable(self) -> Self {
self.enabled(false)
}
}

// Provides a convenient builder pattern to declare Extensions
Expand Down Expand Up @@ -138,6 +163,7 @@ impl ExtensionBuilder {
middleware_fn: self.middleware.take(),
event_loop_middleware: self.event_loop_middleware.take(),
initialized: false,
enabled: true,
}
}
}
Expand Down
35 changes: 34 additions & 1 deletion core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use crate::modules::ModuleLoadId;
use crate::modules::ModuleLoader;
use crate::modules::ModuleMap;
use crate::modules::NoopModuleLoader;
use crate::op_void_async;
use crate::op_void_sync;
use crate::ops::*;
use crate::Extension;
use crate::OpMiddlewareFn;
Expand Down Expand Up @@ -472,7 +474,7 @@ impl JsRuntime {
// macroware wraps an opfn in all the middleware
let macroware = move |d| middleware.iter().fold(d, |d, m| m(d));

// Flatten ops & apply middlware
// Flatten ops, apply middlware & override disabled ops
extensions
.iter_mut()
.filter_map(|e| e.init_ops())
Expand All @@ -481,6 +483,16 @@ impl JsRuntime {
name: d.name,
..macroware(d)
})
.map(|op| match op.enabled {
true => op,
false => OpDecl {
v8_fn_ptr: match op.is_async {
true => op_void_async::v8_fn_ptr(),
false => op_void_sync::v8_fn_ptr(),
},
..op
},
})
.collect()
}

Expand Down Expand Up @@ -3019,4 +3031,25 @@ assertEquals(1, notify_return_value);
let scope = &mut runtime.handle_scope();
assert_eq!(r.open(scope).integer_value(scope), Some(10));
}

#[test]
fn test_op_disabled() {
#[op]
fn op_foo() -> Result<i64, anyhow::Error> {
Ok(42)
}

let ext = Extension::builder()
.ops(vec![op_foo::decl().disable()])
.build();
let mut runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![ext],
..Default::default()
});
let r = runtime
.execute_script("test.js", "Deno.core.opSync('op_foo')")
.unwrap();
let scope = &mut runtime.handle_scope();
assert!(r.open(scope).is_undefined());
}
}
5 changes: 4 additions & 1 deletion ops/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ pub fn op(_attr: TokenStream, item: TokenStream) -> TokenStream {

let core = core_import();

let v8_body = if func.sig.asyncness.is_some() {
let is_async = func.sig.asyncness.is_some();
let v8_body = if is_async {
codegen_v8_async(&core, &func)
} else {
codegen_v8_sync(&core, &func)
Expand Down Expand Up @@ -82,6 +83,8 @@ pub fn op(_attr: TokenStream, item: TokenStream) -> TokenStream {
#core::OpDecl {
name: Self::name(),
v8_fn_ptr: Self::v8_fn_ptr::<#type_params>(),
enabled: true,
is_async: #is_async,
}
}

Expand Down
61 changes: 27 additions & 34 deletions runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,55 +386,48 @@ impl WebWorker {
options.root_cert_store.clone(),
options.unsafely_ignore_certificate_errors.clone(),
),
deno_webstorage::init(None).disable(),
deno_broadcast_channel::init(options.broadcast_channel.clone(), unstable),
deno_crypto::init(options.seed),
deno_webgpu::init(unstable),
// ffi
deno_ffi::init::<Permissions>(unstable),
// Permissions ext (worker specific state)
perm_ext,
];

// Runtime ops that are always initialized for WebWorkers
let runtime_exts = vec![
// Runtime ops that are always initialized for WebWorkers
ops::web_worker::init(),
ops::runtime::init(main_module.clone()),
ops::worker_host::init(
options.create_web_worker_cb.clone(),
options.preload_module_cb.clone(),
),
// Extensions providing Deno.* features
ops::fs_events::init().enabled(options.use_deno_namespace),
ops::fs::init().enabled(options.use_deno_namespace),
ops::io::init(),
ops::io::init_stdio().enabled(options.use_deno_namespace),
deno_tls::init().enabled(options.use_deno_namespace),
deno_net::init::<Permissions>(
options.root_cert_store.clone(),
unstable,
options.unsafely_ignore_certificate_errors.clone(),
)
.enabled(options.use_deno_namespace),
ops::os::init(Some(
options
.maybe_exit_code
.expect("Worker has access to OS ops but exit code was not passed."),
))
.enabled(options.use_deno_namespace),
ops::permissions::init().enabled(options.use_deno_namespace),
ops::process::init().enabled(options.use_deno_namespace),
ops::signal::init().enabled(options.use_deno_namespace),
ops::tty::init().enabled(options.use_deno_namespace),
deno_http::init().enabled(options.use_deno_namespace),
ops::http::init().enabled(options.use_deno_namespace),
// Permissions ext (worker specific state)
perm_ext,
];

// Extensions providing Deno.* features
let deno_ns_exts = if options.use_deno_namespace {
vec![
ops::fs_events::init(),
ops::fs::init(),
deno_tls::init(),
deno_net::init::<Permissions>(
options.root_cert_store.clone(),
unstable,
options.unsafely_ignore_certificate_errors.clone(),
),
ops::os::init(Some(options.maybe_exit_code.expect(
"Worker has access to OS ops but exit code was not passed.",
))),
ops::permissions::init(),
ops::process::init(),
ops::signal::init(),
ops::tty::init(),
deno_http::init(),
ops::http::init(),
ops::io::init_stdio(),
]
} else {
vec![]
};

// Append exts
extensions.extend(runtime_exts);
extensions.extend(deno_ns_exts); // May be empty
extensions.extend(std::mem::take(&mut options.extensions));

let mut js_runtime = JsRuntime::new(RuntimeOptions {
Expand Down
2 changes: 1 addition & 1 deletion runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ impl MainWorker {
options.unsafely_ignore_certificate_errors.clone(),
),
deno_webstorage::init(options.origin_storage_dir.clone()),
deno_crypto::init(options.seed),
deno_broadcast_channel::init(options.broadcast_channel.clone(), unstable),
deno_crypto::init(options.seed),
deno_webgpu::init(unstable),
// ffi
deno_ffi::init::<Permissions>(unstable),
Expand Down

0 comments on commit f81334d

Please sign in to comment.