From c464cd7073c761780b3170a48542c387560e3f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 12 Oct 2023 17:55:50 +0200 Subject: [PATCH] refactor: FeatureChecker integration in ext/ crates (#20797) Towards https://github.com/denoland/deno/issues/20779. --- Cargo.lock | 12 ++++++------ Cargo.toml | 2 +- cli/factory.rs | 18 ++++++++++++++++++ cli/main.rs | 13 +++++++++++++ cli/standalone/mod.rs | 13 +++++++++++++ cli/tsc/mod.rs | 2 +- cli/worker.rs | 6 ++++++ ext/broadcast_channel/lib.rs | 11 ++++++++--- ext/ffi/lib.rs | 6 +++++- ext/fs/lib.rs | 6 +++++- ext/http/http_next.rs | 10 +++++++++- ext/kv/lib.rs | 6 +++++- ext/net/lib.rs | 6 +++++- ext/net/ops.rs | 8 ++++---- runtime/ops/http.rs | 4 +++- runtime/ops/mod.rs | 6 ++++-- runtime/ops/process.rs | 8 +++++--- runtime/ops/worker_host.rs | 8 +++++++- runtime/web_worker.rs | 12 +++--------- runtime/worker.rs | 13 ++++--------- 20 files changed, 125 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8afda07c2b..010e41b61e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1205,9 +1205,9 @@ dependencies = [ [[package]] name = "deno_core" -version = "0.221.0" +version = "0.222.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf694d66a55050bdfad2036765f93547392ba7c131adc7fcd5e91fa31e291c51" +checksum = "b13c81b9ea8462680e7b77088a44fc36390bab3dbfa5a205a285e11b64e0919c" dependencies = [ "anyhow", "bytes", @@ -1594,9 +1594,9 @@ dependencies = [ [[package]] name = "deno_ops" -version = "0.97.0" +version = "0.98.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aab46b645231decaf3b03d9eb99a5207e4e66c8c53b7774000b44e3ae43e0f99" +checksum = "bf89da1a3e50ff7c89956495b53d9bcad29e1f1b3f3d2bc54cad7155f55419c4" dependencies = [ "deno-proc-macro-rules", "lazy-regex", @@ -4821,9 +4821,9 @@ dependencies = [ [[package]] name = "serde_v8" -version = "0.130.0" +version = "0.131.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c6078fc5e16615b092b26199e7f1d7d2d336848623d32e00f95e74618274111" +checksum = "38cafa16d0a4288d75925351bb54d06d2e830118ad3fad393947bb11f91b18f3" dependencies = [ "bytes", "derive_more", diff --git a/Cargo.toml b/Cargo.toml index 3720eb74f2..6dcdc787aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ repository = "https://github.com/denoland/deno" [workspace.dependencies] deno_ast = { version = "0.29.3", features = ["transpiling"] } -deno_core = { version = "0.221.0" } +deno_core = { version = "0.222.0" } deno_runtime = { version = "0.128.0", path = "./runtime" } napi_sym = { version = "0.50.0", path = "./cli/napi/sym" } diff --git a/cli/factory.rs b/cli/factory.rs index 14f448b19f..e4f9b60fe4 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -47,6 +47,7 @@ use crate::worker::CliMainWorkerOptions; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; +use deno_core::FeatureChecker; use deno_graph::GraphKind; use deno_runtime::deno_fs; @@ -161,6 +162,7 @@ struct CliFactoryServices { type_checker: Deferred>, cjs_resolutions: Deferred>, cli_node_resolver: Deferred>, + feature_checker: Deferred>, } pub struct CliFactory { @@ -558,6 +560,21 @@ impl CliFactory { .await } + pub fn feature_checker(&self) -> &Arc { + self.services.feature_checker.get_or_init(|| { + let mut checker = FeatureChecker::default(); + checker.set_exit_cb(Box::new(crate::unstable_exit_cb)); + // TODO(bartlomieju): enable, once we deprecate `--unstable` in favor + // of granular --unstable-* flags. + // feature_checker.set_warn_cb(Box::new(crate::unstable_warn_cb)); + if self.options.unstable() { + checker.enable_legacy_unstable(); + } + + Arc::new(checker) + }) + } + pub async fn create_compile_binary_writer( &self, ) -> Result { @@ -602,6 +619,7 @@ impl CliFactory { self.fs().clone(), self.maybe_inspector_server().clone(), self.maybe_lockfile().clone(), + self.feature_checker().clone(), self.create_cli_main_worker_options()?, )) } diff --git a/cli/main.rs b/cli/main.rs index 1ccd694ee9..0817c0984c 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -256,6 +256,19 @@ fn unwrap_or_exit(result: Result) -> T { } } +pub(crate) fn unstable_exit_cb(_feature: &str, api_name: &str) { + // TODO(bartlomieju): change to "The `--unstable-{feature}` flag must be provided.". + eprintln!("Unstable API '{api_name}'. The --unstable flag must be provided."); + std::process::exit(70); +} + +#[allow(dead_code)] +pub(crate) fn unstable_warn_cb(feature: &str) { + eprintln!( + "The `--unstable` flag is deprecated, use --unstable-{feature} instead." + ); +} + pub fn main() { setup_panic_hook(); diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index e537f9e0c6..612ae9eedd 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -35,6 +35,7 @@ use deno_core::error::type_error; use deno_core::error::AnyError; use deno_core::futures::FutureExt; use deno_core::v8_set_flags; +use deno_core::FeatureChecker; use deno_core::ModuleLoader; use deno_core::ModuleSpecifier; use deno_core::ModuleType; @@ -424,6 +425,17 @@ pub async fn run( PermissionsContainer::new(Permissions::from_options(&permissions)?) }; + let feature_checker = Arc::new({ + let mut checker = FeatureChecker::default(); + checker.set_exit_cb(Box::new(crate::unstable_exit_cb)); + // TODO(bartlomieju): enable, once we deprecate `--unstable` in favor + // of granular --unstable-* flags. + // feature_checker.set_warn_cb(Box::new(crate::unstable_warn_cb)); + if metadata.unstable { + checker.enable_legacy_unstable(); + } + checker + }); let worker_factory = CliMainWorkerFactory::new( StorageKeyResolver::empty(), npm_resolver, @@ -434,6 +446,7 @@ pub async fn run( fs, None, None, + feature_checker, CliMainWorkerOptions { argv: metadata.argv, log_level: WorkerLogLevel::Info, diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 5f4f7ddf16..5247197556 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -948,7 +948,7 @@ mod tests { .context("Unable to get CWD") .unwrap(), ); - let mut op_state = OpState::new(1); + let mut op_state = OpState::new(1, None); op_state.put(state); op_state } diff --git a/cli/worker.rs b/cli/worker.rs index de8cb2018d..d8738d4921 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -16,6 +16,7 @@ use deno_core::url::Url; use deno_core::v8; use deno_core::CompiledWasmModuleStore; use deno_core::Extension; +use deno_core::FeatureChecker; use deno_core::ModuleId; use deno_core::ModuleLoader; use deno_core::SharedArrayBufferStore; @@ -109,6 +110,7 @@ struct SharedWorkerState { fs: Arc, maybe_inspector_server: Option>, maybe_lockfile: Option>>, + feature_checker: Arc, } impl SharedWorkerState { @@ -313,6 +315,7 @@ impl CliMainWorkerFactory { fs: Arc, maybe_inspector_server: Option>, maybe_lockfile: Option>>, + feature_checker: Arc, options: CliMainWorkerOptions, ) -> Self { Self { @@ -330,6 +333,7 @@ impl CliMainWorkerFactory { fs, maybe_inspector_server, maybe_lockfile, + feature_checker, }), } } @@ -510,6 +514,7 @@ impl CliMainWorkerFactory { shared.compiled_wasm_module_store.clone(), ), stdio, + feature_checker: shared.feature_checker.clone(), }; let worker = MainWorker::bootstrap_from_options( @@ -681,6 +686,7 @@ fn create_web_worker_callback( ), stdio: stdio.clone(), cache_storage_dir, + feature_checker: shared.feature_checker.clone(), }; WebWorker::bootstrap_from_options( diff --git a/ext/broadcast_channel/lib.rs b/ext/broadcast_channel/lib.rs index 49cc784159..6079a83102 100644 --- a/ext/broadcast_channel/lib.rs +++ b/ext/broadcast_channel/lib.rs @@ -17,6 +17,8 @@ use deno_core::OpState; use deno_core::Resource; use deno_core::ResourceId; +pub const UNSTABLE_FEATURE_NAME: &str = "broadcast-channel"; + #[async_trait] pub trait BroadcastChannel: Clone { type Resource: Resource; @@ -48,9 +50,12 @@ pub fn op_broadcast_subscribe( where BC: BroadcastChannel + 'static, { - state - .feature_checker - .check_legacy_unstable_or_exit("BroadcastChannel"); + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` + state.feature_checker.check_or_exit_with_legacy_fallback( + UNSTABLE_FEATURE_NAME, + "BroadcastChannel", + ); let bc = state.borrow::(); let resource = bc.subscribe()?; Ok(state.resource_table.add(resource)) diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index a3cd0ebdae..0541e1eeb0 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -45,10 +45,14 @@ const _: () = { pub(crate) const MAX_SAFE_INTEGER: isize = 9007199254740991; pub(crate) const MIN_SAFE_INTEGER: isize = -9007199254740991; +pub const UNSTABLE_FEATURE_NAME: &str = "ffi"; + fn check_unstable(state: &OpState, api_name: &str) { + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .feature_checker - .check_legacy_unstable_or_exit(api_name); + .check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name) } pub trait FfiPermissions { diff --git a/ext/fs/lib.rs b/ext/fs/lib.rs index ab19540b5b..d36d8b5c2d 100644 --- a/ext/fs/lib.rs +++ b/ext/fs/lib.rs @@ -64,11 +64,15 @@ pub trait FsPermissions { } } +pub const UNSTABLE_FEATURE_NAME: &str = "fs"; + /// Helper for checking unstable features. Used for sync ops. fn check_unstable(state: &OpState, api_name: &str) { + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .feature_checker - .check_legacy_unstable_or_exit(api_name); + .check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name); } deno_core::extension!(deno_fs, diff --git a/ext/http/http_next.rs b/ext/http/http_next.rs index 90f7f0b1db..522df280f6 100644 --- a/ext/http/http_next.rs +++ b/ext/http/http_next.rs @@ -83,6 +83,8 @@ static USE_WRITEV: Lazy = Lazy::new(|| { false }); +pub const UNSTABLE_FEATURE_NAME: &str = "http"; + /// All HTTP/2 connections start with this byte string. /// /// In HTTP/2, each endpoint is required to send a connection preface as a final confirmation @@ -1105,10 +1107,16 @@ pub async fn op_http_close( .take::(rid)?; if graceful { + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .borrow() .feature_checker - .check_legacy_unstable_or_exit("Deno.Server.shutdown"); + .check_or_exit_with_legacy_fallback( + UNSTABLE_FEATURE_NAME, + "Deno.Server.shutdown", + ); + // In a graceful shutdown, we close the listener and allow all the remaining connections to drain join_handle.listen_cancel_handle().cancel(); } else { diff --git a/ext/kv/lib.rs b/ext/kv/lib.rs index 3056e4660f..e99b14552b 100644 --- a/ext/kv/lib.rs +++ b/ext/kv/lib.rs @@ -33,6 +33,8 @@ use serde::Serialize; pub use crate::interface::*; +pub const UNSTABLE_FEATURE_NAME: &str = "kv"; + const MAX_WRITE_KEY_SIZE_BYTES: usize = 2048; // range selectors can contain 0x00 or 0xff suffixes const MAX_READ_KEY_SIZE_BYTES: usize = MAX_WRITE_KEY_SIZE_BYTES + 1; @@ -89,9 +91,11 @@ where { let handler = { let state = state.borrow(); + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .feature_checker - .check_legacy_unstable_or_exit("Deno.openKv"); + .check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, "Deno.openKv"); state.borrow::>().clone() }; let db = handler.open(state.clone(), path).await?; diff --git a/ext/net/lib.rs b/ext/net/lib.rs index 1fc7e34205..fb8dda514c 100644 --- a/ext/net/lib.rs +++ b/ext/net/lib.rs @@ -16,6 +16,8 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +pub const UNSTABLE_FEATURE_NAME: &str = "net"; + pub trait NetPermissions { fn check_net>( &mut self, @@ -29,9 +31,11 @@ pub trait NetPermissions { /// Helper for checking unstable features. Used for sync ops. fn check_unstable(state: &OpState, api_name: &str) { + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .feature_checker - .check_legacy_unstable_or_exit(api_name); + .check_or_exit_with_legacy_fallback(UNSTABLE_FEATURE_NAME, api_name); } pub fn get_declaration() -> PathBuf { diff --git a/ext/net/ops.rs b/ext/net/ops.rs index 5738620f8c..bbf8975554 100644 --- a/ext/net/ops.rs +++ b/ext/net/ops.rs @@ -1041,16 +1041,16 @@ mod tests { } ); + let mut feature_checker = deno_core::FeatureChecker::default(); + feature_checker.enable_legacy_unstable(); + let mut runtime = JsRuntime::new(RuntimeOptions { extensions: vec![test_ext::init_ops()], + feature_checker: Some(Arc::new(feature_checker)), ..Default::default() }); let conn_state = runtime.op_state(); - conn_state - .borrow_mut() - .feature_checker - .enable_legacy_unstable(); let server_addr: Vec<&str> = clone_addr.split(':').collect(); let ip_addr = IpAddr { diff --git a/runtime/ops/http.rs b/runtime/ops/http.rs index 07757850cd..f0f0510daf 100644 --- a/runtime/ops/http.rs +++ b/runtime/ops/http.rs @@ -27,6 +27,8 @@ use deno_net::io::UnixStreamResource; #[cfg(unix)] use tokio::net::UnixStream; +pub const UNSTABLE_FEATURE_NAME: &str = "http"; + deno_core::extension!( deno_http_runtime, ops = [op_http_start, op_http_upgrade], @@ -73,7 +75,7 @@ fn op_http_start( .resource_table .take::(tcp_stream_rid) { - super::check_unstable(state, "Deno.serveHttp"); + super::check_unstable(state, UNSTABLE_FEATURE_NAME, "Deno.serveHttp"); // This UNIX socket might be used somewhere else. If it's the case, we cannot proceed with the // process of starting a HTTP server on top of this UNIX socket, so we just return a bad diff --git a/runtime/ops/mod.rs b/runtime/ops/mod.rs index cdafd61ecd..bafa6c5718 100644 --- a/runtime/ops/mod.rs +++ b/runtime/ops/mod.rs @@ -15,10 +15,12 @@ pub mod worker_host; use deno_core::OpState; /// Helper for checking unstable features. Used for sync ops. -pub fn check_unstable(state: &OpState, api_name: &str) { +pub fn check_unstable(state: &OpState, feature: &str, api_name: &str) { + // TODO(bartlomieju): replace with `state.feature_checker.check_or_exit` + // once we phase out `check_or_exit_with_legacy_fallback` state .feature_checker - .check_legacy_unstable_or_exit(api_name); + .check_or_exit_with_legacy_fallback(feature, api_name); } pub struct TestingFeaturesEnabled(pub bool); diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index 51c9f961a4..1fdd4bf4d5 100644 --- a/runtime/ops/process.rs +++ b/runtime/ops/process.rs @@ -34,6 +34,8 @@ use std::os::unix::prelude::ExitStatusExt; #[cfg(unix)] use std::os::unix::process::CommandExt; +pub const UNSTABLE_FEATURE_NAME: &str = "process"; + #[derive(Copy, Clone, Eq, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] pub enum Stdio { @@ -503,7 +505,7 @@ mod deprecated { cwd.map(|d| c.current_dir(d)); if run_args.clear_env { - super::check_unstable(state, "Deno.run.clearEnv"); + super::check_unstable(state, UNSTABLE_FEATURE_NAME, "Deno.run.clearEnv"); c.env_clear(); } for (key, value) in &env { @@ -512,12 +514,12 @@ mod deprecated { #[cfg(unix)] if let Some(gid) = run_args.gid { - super::check_unstable(state, "Deno.run.gid"); + super::check_unstable(state, UNSTABLE_FEATURE_NAME, "Deno.run.gid"); c.gid(gid); } #[cfg(unix)] if let Some(uid) = run_args.uid { - super::check_unstable(state, "Deno.run.uid"); + super::check_unstable(state, UNSTABLE_FEATURE_NAME, "Deno.run.uid"); c.uid(uid); } #[cfg(unix)] diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs index a77f57b60f..960e35b3d4 100644 --- a/runtime/ops/worker_host.rs +++ b/runtime/ops/worker_host.rs @@ -26,6 +26,8 @@ use std::collections::HashMap; use std::rc::Rc; use std::sync::Arc; +pub const UNSTABLE_FEATURE_NAME: &str = "worker"; + pub struct CreateWebWorkerArgs { pub name: String, pub worker_id: WorkerId, @@ -140,7 +142,11 @@ fn op_create_worker( } if args.permissions.is_some() { - super::check_unstable(state, "Worker.deno.permissions"); + super::check_unstable( + state, + UNSTABLE_FEATURE_NAME, + "Worker.deno.permissions", + ); } let parent_permissions = state.borrow_mut::(); let worker_permissions = if let Some(child_permissions_arg) = args.permissions diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index f37a77d889..c1fe5a6199 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -25,6 +25,7 @@ use deno_core::v8; use deno_core::CancelHandle; use deno_core::CompiledWasmModuleStore; use deno_core::Extension; +use deno_core::FeatureChecker; use deno_core::GetErrorClassFn; use deno_core::JsRuntime; use deno_core::ModuleCode; @@ -349,6 +350,7 @@ pub struct WebWorkerOptions { pub compiled_wasm_module_store: Option, pub cache_storage_dir: Option, pub stdio: Stdio, + pub feature_checker: Arc, } impl WebWorker { @@ -385,7 +387,6 @@ impl WebWorker { ); // Permissions: many ops depend on this - let unstable = options.bootstrap.unstable; let enable_testing_features = options.bootstrap.enable_testing_features; let create_cache = options.cache_storage_dir.map(|storage_dir| { let create_cache_fn = move || SqliteBackedCache::new(storage_dir.clone()); @@ -509,17 +510,10 @@ impl WebWorker { extensions, inspector: options.maybe_inspector_server.is_some(), preserve_snapshotted_modules, + feature_checker: Some(options.feature_checker.clone()), ..Default::default() }); - if unstable { - let op_state = js_runtime.op_state(); - op_state - .borrow_mut() - .feature_checker - .enable_legacy_unstable(); - } - if let Some(server) = options.maybe_inspector_server.clone() { server.register_inspector( main_module.to_string(), diff --git a/runtime/worker.rs b/runtime/worker.rs index e8874575aa..e8d9ca6bcc 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -18,6 +18,7 @@ use deno_core::futures::Future; use deno_core::v8; use deno_core::CompiledWasmModuleStore; use deno_core::Extension; +use deno_core::FeatureChecker; use deno_core::FsModuleLoader; use deno_core::GetErrorClassFn; use deno_core::JsRuntime; @@ -141,6 +142,7 @@ pub struct WorkerOptions { /// `WebAssembly.Module` objects cannot be serialized. pub compiled_wasm_module_store: Option, pub stdio: Stdio, + pub feature_checker: Arc, } impl Default for WorkerOptions { @@ -172,6 +174,7 @@ impl Default for WorkerOptions { create_params: Default::default(), bootstrap: Default::default(), stdio: Default::default(), + feature_checker: Default::default(), } } } @@ -205,7 +208,6 @@ impl MainWorker { ); // Permissions: many ops depend on this - let unstable = options.bootstrap.unstable; let enable_testing_features = options.bootstrap.enable_testing_features; let exit_code = ExitCode(Arc::new(AtomicI32::new(0))); let create_cache = options.cache_storage_dir.map(|storage_dir| { @@ -334,17 +336,10 @@ impl MainWorker { preserve_snapshotted_modules, inspector: options.maybe_inspector_server.is_some(), is_main: true, + feature_checker: Some(options.feature_checker.clone()), ..Default::default() }); - if unstable { - let op_state = js_runtime.op_state(); - op_state - .borrow_mut() - .feature_checker - .enable_legacy_unstable(); - } - if let Some(server) = options.maybe_inspector_server.clone() { server.register_inspector( main_module.to_string(),