diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-09-27 12:35:37 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-09-27 12:35:37 -0700 |
commit | fbddd5a2ebfb11dd376a751e9fc4cf09a6286ada (patch) | |
tree | 75c13ee9f26f61fe8c1d6f80df2580a523177c1b | |
parent | a8d1ab52761516b7f9b6069d6e433254794ed48c (diff) |
fix(node): Pass NPM_PROCESS_STATE to subprocesses via temp file instead of env var (#25896)
Fixes https://github.com/denoland/deno/issues/25401. Fixes
https://github.com/denoland/deno/issues/25841. Fixes
https://github.com/denoland/deno/issues/25891.
-rw-r--r-- | Cargo.lock | 1 | ||||
-rw-r--r-- | cli/args/mod.rs | 36 | ||||
-rw-r--r-- | cli/npm/byonm.rs | 2 | ||||
-rw-r--r-- | cli/npm/managed/mod.rs | 2 | ||||
-rw-r--r-- | cli/npm/managed/resolvers/common/lifecycle_scripts.rs | 21 | ||||
-rw-r--r-- | cli/npm/mod.rs | 2 | ||||
-rw-r--r-- | cli/worker.rs | 11 | ||||
-rw-r--r-- | ext/io/bi_pipe.rs | 6 | ||||
-rw-r--r-- | ext/io/lib.rs | 107 | ||||
-rw-r--r-- | ext/io/pipe.rs | 2 | ||||
-rw-r--r-- | ext/node/lib.rs | 29 | ||||
-rw-r--r-- | ext/node/polyfills/child_process.ts | 7 | ||||
-rw-r--r-- | ext/node/polyfills/internal/child_process.ts | 9 | ||||
-rw-r--r-- | runtime/Cargo.toml | 1 | ||||
-rw-r--r-- | runtime/js/40_process.js | 4 | ||||
-rw-r--r-- | runtime/js/99_main.js | 1 | ||||
-rw-r--r-- | runtime/ops/process.rs | 106 | ||||
-rw-r--r-- | runtime/snapshot.rs | 2 | ||||
-rw-r--r-- | runtime/web_worker.rs | 6 | ||||
-rw-r--r-- | runtime/worker.rs | 7 | ||||
-rw-r--r-- | tests/registry/npm/@denotest/child-process-fork/1.0.0/index.js | 11 | ||||
-rw-r--r-- | tests/unit/ops_test.ts | 2 |
22 files changed, 288 insertions, 87 deletions
diff --git a/Cargo.lock b/Cargo.lock index bc51e67b8..206de55a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1985,6 +1985,7 @@ dependencies = [ "serde", "signal-hook", "signal-hook-registry", + "tempfile", "test_server", "tokio", "tokio-metrics", diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 995a04823..c3a4c2937 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -69,6 +69,8 @@ use std::collections::HashMap; use std::env; use std::io::BufReader; use std::io::Cursor; +use std::io::Read; +use std::io::Seek; use std::net::SocketAddr; use std::num::NonZeroUsize; use std::path::Path; @@ -742,15 +744,33 @@ pub enum NpmProcessStateKind { Byonm, } -pub(crate) const NPM_RESOLUTION_STATE_ENV_VAR_NAME: &str = - "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE"; - static NPM_PROCESS_STATE: Lazy<Option<NpmProcessState>> = Lazy::new(|| { - let state = std::env::var(NPM_RESOLUTION_STATE_ENV_VAR_NAME).ok()?; - let state: NpmProcessState = serde_json::from_str(&state).ok()?; - // remove the environment variable so that sub processes - // that are spawned do not also use this. - std::env::remove_var(NPM_RESOLUTION_STATE_ENV_VAR_NAME); + use deno_runtime::ops::process::NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME; + let fd = std::env::var(NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME).ok()?; + std::env::remove_var(NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME); + let fd = fd.parse::<usize>().ok()?; + let mut file = { + use deno_runtime::deno_io::FromRawIoHandle; + unsafe { std::fs::File::from_raw_io_handle(fd as _) } + }; + let mut buf = Vec::new(); + // seek to beginning. after the file is written the position will be inherited by this subprocess, + // and also this file might have been read before + file.seek(std::io::SeekFrom::Start(0)).unwrap(); + file + .read_to_end(&mut buf) + .inspect_err(|e| { + log::error!("failed to read npm process state from fd {fd}: {e}"); + }) + .ok()?; + let state: NpmProcessState = serde_json::from_slice(&buf) + .inspect_err(|e| { + log::error!( + "failed to deserialize npm process state: {e} {}", + String::from_utf8_lossy(&buf) + ) + }) + .ok()?; Some(state) }); diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index fc7069e1f..4c8bbd394 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -14,9 +14,9 @@ use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::DenoPkgJsonFsAdapter; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeRequireResolver; -use deno_runtime::deno_node::NpmProcessStateProvider; use deno_runtime::deno_node::PackageJson; use deno_runtime::fs_util::specifier_to_file_path; +use deno_runtime::ops::process::NpmProcessStateProvider; use deno_semver::package::PackageReq; use deno_semver::Version; use node_resolver::errors::PackageFolderResolveError; diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index 0be26b785..40c92cd46 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -22,7 +22,7 @@ use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeRequireResolver; -use deno_runtime::deno_node::NpmProcessStateProvider; +use deno_runtime::ops::process::NpmProcessStateProvider; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use node_resolver::errors::PackageFolderResolveError; diff --git a/cli/npm/managed/resolvers/common/lifecycle_scripts.rs b/cli/npm/managed/resolvers/common/lifecycle_scripts.rs index f700d4bf9..b358c3585 100644 --- a/cli/npm/managed/resolvers/common/lifecycle_scripts.rs +++ b/cli/npm/managed/resolvers/common/lifecycle_scripts.rs @@ -2,7 +2,9 @@ use super::bin_entries::BinEntries; use crate::args::LifecycleScriptsConfig; +use deno_core::anyhow::Context; use deno_npm::resolution::NpmResolutionSnapshot; +use deno_runtime::deno_io::FromRawIoHandle; use deno_semver::package::PackageNv; use deno_semver::Version; use std::borrow::Cow; @@ -163,9 +165,24 @@ impl<'a> LifecycleScripts<'a> { ); let mut env_vars = crate::task_runner::real_env_vars(); + // we want to pass the current state of npm resolution down to the deno subprocess + // (that may be running as part of the script). we do this with an inherited temp file + // + // SAFETY: we are sharing a single temp file across all of the scripts. the file position + // will be shared among these, which is okay since we run only one script at a time. + // However, if we concurrently run scripts in the future we will + // have to have multiple temp files. + let temp_file_fd = + deno_runtime::ops::process::npm_process_state_tempfile( + process_state.as_bytes(), + ).context("failed to create npm process state tempfile for running lifecycle scripts")?; + // SAFETY: fd/handle is valid + let _temp_file = + unsafe { std::fs::File::from_raw_io_handle(temp_file_fd) }; // make sure the file gets closed env_vars.insert( - crate::args::NPM_RESOLUTION_STATE_ENV_VAR_NAME.to_string(), - process_state, + deno_runtime::ops::process::NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME + .to_string(), + (temp_file_fd as usize).to_string(), ); for (package, package_path) in self.packages_with_scripts { // add custom commands for binaries from the package's dependencies. this will take precedence over the diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index bedde6455..15ac8ebb2 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -14,7 +14,7 @@ use deno_core::error::AnyError; use deno_core::serde_json; use deno_npm::registry::NpmPackageInfo; use deno_runtime::deno_node::NodeRequireResolver; -use deno_runtime::deno_node::NpmProcessStateProvider; +use deno_runtime::ops::process::NpmProcessStateProvider; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use node_resolver::NpmResolver; diff --git a/cli/worker.rs b/cli/worker.rs index 861419f1e..c355d18bd 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -29,6 +29,7 @@ use deno_runtime::deno_tls::RootCertStoreProvider; use deno_runtime::deno_web::BlobStore; use deno_runtime::fmt_errors::format_js_error; use deno_runtime::inspector_server::InspectorServer; +use deno_runtime::ops::process::NpmProcessStateProviderRc; use deno_runtime::ops::worker_host::CreateWebWorkerCb; use deno_runtime::permissions::RuntimePermissionDescriptorParser; use deno_runtime::web_worker::WebWorker; @@ -147,13 +148,13 @@ impl SharedWorkerState { NodeExtInitServices { node_require_resolver: self.npm_resolver.clone().into_require_resolver(), node_resolver: self.node_resolver.clone(), - npm_process_state_provider: self - .npm_resolver - .clone() - .into_process_state_provider(), npm_resolver: self.npm_resolver.clone().into_npm_resolver(), } } + + pub fn npm_process_state_provider(&self) -> NpmProcessStateProviderRc { + self.npm_resolver.clone().into_process_state_provider() + } } pub struct CliMainWorker { @@ -614,6 +615,7 @@ impl CliMainWorkerFactory { module_loader, fs: shared.fs.clone(), node_services: Some(shared.create_node_init_services()), + npm_process_state_provider: Some(shared.npm_process_state_provider()), get_error_class_fn: Some(&errors::get_error_class_name), cache_storage_dir, origin_storage_dir, @@ -820,6 +822,7 @@ fn create_web_worker_callback( strace_ops: shared.options.strace_ops.clone(), close_on_idle: args.close_on_idle, maybe_worker_metadata: args.maybe_worker_metadata, + npm_process_state_provider: Some(shared.npm_process_state_provider()), }; WebWorker::bootstrap_from_options( diff --git a/ext/io/bi_pipe.rs b/ext/io/bi_pipe.rs index 04fff7b00..402e383ac 100644 --- a/ext/io/bi_pipe.rs +++ b/ext/io/bi_pipe.rs @@ -11,11 +11,7 @@ use deno_core::RcRef; use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; -#[cfg(unix)] -pub type RawBiPipeHandle = std::os::fd::RawFd; - -#[cfg(windows)] -pub type RawBiPipeHandle = std::os::windows::io::RawHandle; +pub type RawBiPipeHandle = super::RawIoHandle; /// One end of a bidirectional pipe. This implements the /// `Resource` trait. diff --git a/ext/io/lib.rs b/ext/io/lib.rs index 47921bcee..a07d64ae3 100644 --- a/ext/io/lib.rs +++ b/ext/io/lib.rs @@ -67,6 +67,7 @@ pub use pipe::AsyncPipeRead; pub use pipe::AsyncPipeWrite; pub use pipe::PipeRead; pub use pipe::PipeWrite; +pub use pipe::RawPipeHandle; pub use bi_pipe::bi_pipe_pair_raw; pub use bi_pipe::BiPipe; @@ -75,6 +76,112 @@ pub use bi_pipe::BiPipeResource; pub use bi_pipe::BiPipeWrite; pub use bi_pipe::RawBiPipeHandle; +/// Abstraction over `AsRawFd` (unix) and `AsRawHandle` (windows) +pub trait AsRawIoHandle { + fn as_raw_io_handle(&self) -> RawIoHandle; +} + +#[cfg(unix)] +impl<T> AsRawIoHandle for T +where + T: std::os::unix::io::AsRawFd, +{ + fn as_raw_io_handle(&self) -> RawIoHandle { + self.as_raw_fd() + } +} + +#[cfg(windows)] +impl<T> AsRawIoHandle for T +where + T: std::os::windows::io::AsRawHandle, +{ + fn as_raw_io_handle(&self) -> RawIoHandle { + self.as_raw_handle() + } +} + +/// Abstraction over `IntoRawFd` (unix) and `IntoRawHandle` (windows) +pub trait IntoRawIoHandle { + fn into_raw_io_handle(self) -> RawIoHandle; +} + +#[cfg(unix)] +impl<T> IntoRawIoHandle for T +where + T: std::os::unix::io::IntoRawFd, +{ + fn into_raw_io_handle(self) -> RawIoHandle { + self.into_raw_fd() + } +} + +#[cfg(windows)] +impl<T> IntoRawIoHandle for T +where + T: std::os::windows::io::IntoRawHandle, +{ + fn into_raw_io_handle(self) -> RawIoHandle { + self.into_raw_handle() + } +} + +/// Abstraction over `FromRawFd` (unix) and `FromRawHandle` (windows) +pub trait FromRawIoHandle: Sized { + /// Constructs a type from a raw io handle (fd/HANDLE). + /// + /// # Safety + /// + /// Refer to the standard library docs ([unix](https://doc.rust-lang.org/stable/std/os/windows/io/trait.FromRawHandle.html#tymethod.from_raw_handle)) ([windows](https://doc.rust-lang.org/stable/std/os/fd/trait.FromRawFd.html#tymethod.from_raw_fd)) + /// + unsafe fn from_raw_io_handle(handle: RawIoHandle) -> Self; +} + +#[cfg(unix)] +impl<T> FromRawIoHandle for T +where + T: std::os::unix::io::FromRawFd, +{ + unsafe fn from_raw_io_handle(fd: RawIoHandle) -> T { + // SAFETY: upheld by caller + unsafe { T::from_raw_fd(fd) } + } +} + +#[cfg(windows)] +impl<T> FromRawIoHandle for T +where + T: std::os::windows::io::FromRawHandle, +{ + unsafe fn from_raw_io_handle(fd: RawIoHandle) -> T { + // SAFETY: upheld by caller + unsafe { T::from_raw_handle(fd) } + } +} + +#[cfg(unix)] +pub type RawIoHandle = std::os::fd::RawFd; + +#[cfg(windows)] +pub type RawIoHandle = std::os::windows::io::RawHandle; + +pub fn close_raw_handle(handle: RawIoHandle) { + #[cfg(unix)] + { + // SAFETY: libc call + unsafe { + libc::close(handle); + } + } + #[cfg(windows)] + { + // SAFETY: win32 call + unsafe { + windows_sys::Win32::Foundation::CloseHandle(handle as _); + } + } +} + // Store the stdio fd/handles in global statics in order to keep them // alive for the duration of the application since the last handle/fd // being dropped will close the corresponding pipe. diff --git a/ext/io/pipe.rs b/ext/io/pipe.rs index 70788f752..e0e019e27 100644 --- a/ext/io/pipe.rs +++ b/ext/io/pipe.rs @@ -3,6 +3,8 @@ use std::io; use std::pin::Pin; use std::process::Stdio; +pub type RawPipeHandle = super::RawIoHandle; + // The synchronous read end of a unidirectional pipe. pub struct PipeRead { file: std::fs::File, diff --git a/ext/node/lib.rs b/ext/node/lib.rs index af14e3e85..0c821ecf8 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -16,7 +16,6 @@ use deno_core::url::Url; use deno_core::v8; use deno_core::v8::ExternalReference; use deno_core::JsRuntime; -use deno_core::OpState; use deno_fs::sync::MaybeSend; use deno_fs::sync::MaybeSync; use node_resolver::NpmResolverRc; @@ -121,24 +120,6 @@ impl NodePermissions for deno_permissions::PermissionsContainer { } #[allow(clippy::disallowed_types)] -pub type NpmProcessStateProviderRc = - deno_fs::sync::MaybeArc<dyn NpmProcessStateProvider>; - -pub trait NpmProcessStateProvider: - std::fmt::Debug + MaybeSend + MaybeSync -{ - /// Gets a string containing the serialized npm state of the process. - /// - /// This will be set on the `DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE` environment - /// variable when doing a `child_process.fork`. The implementor can then check this environment - /// variable on startup to repopulate the internal npm state. - fn get_npm_process_state(&self) -> String { - // This method is only used in the CLI. - String::new() - } -} - -#[allow(clippy::disallowed_types)] pub type NodeRequireResolverRc = deno_fs::sync::MaybeArc<dyn NodeRequireResolver>; @@ -165,17 +146,9 @@ fn op_node_build_os() -> String { env!("TARGET").split('-').nth(2).unwrap().to_string() } -#[op2] -#[string] -fn op_npm_process_state(state: &mut OpState) -> Result<String, AnyError> { - let npm_resolver = state.borrow_mut::<NpmProcessStateProviderRc>(); - Ok(npm_resolver.get_npm_process_state()) -} - pub struct NodeExtInitServices { pub node_require_resolver: NodeRequireResolverRc, pub node_resolver: NodeResolverRc, - pub npm_process_state_provider: NpmProcessStateProviderRc, pub npm_resolver: NpmResolverRc, } @@ -374,7 +347,6 @@ deno_core::extension!(deno_node, ops::os::op_cpus<P>, ops::os::op_homedir<P>, op_node_build_os, - op_npm_process_state, ops::require::op_require_can_parse_as_esm, ops::require::op_require_init_paths, ops::require::op_require_node_module_paths<P>, @@ -662,7 +634,6 @@ deno_core::extension!(deno_node, state.put(init.node_require_resolver.clone()); state.put(init.node_resolver.clone()); state.put(init.npm_resolver.clone()); - state.put(init.npm_process_state_provider.clone()); } }, global_template_middleware = global_template_middleware, diff --git a/ext/node/polyfills/child_process.ts b/ext/node/polyfills/child_process.ts index f77a430c2..c37dfc410 100644 --- a/ext/node/polyfills/child_process.ts +++ b/ext/node/polyfills/child_process.ts @@ -10,7 +10,6 @@ import { internals } from "ext:core/mod.js"; import { op_bootstrap_unstable_args, op_node_child_ipc_pipe, - op_npm_process_state, } from "ext:core/ops"; import { @@ -54,6 +53,7 @@ import { convertToValidSignal, kEmptyObject, } from "ext:deno_node/internal/util.mjs"; +import { kNeedsNpmProcessState } from "ext:runtime/40_process.js"; const MAX_BUFFER = 1024 * 1024; @@ -168,9 +168,8 @@ export function fork( options.execPath = options.execPath || Deno.execPath(); options.shell = false; - Object.assign(options.env ??= {}, { - DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE: op_npm_process_state(), - }); + // deno-lint-ignore no-explicit-any + (options as any)[kNeedsNpmProcessState] = true; return spawn(options.execPath, args, options); } diff --git a/ext/node/polyfills/internal/child_process.ts b/ext/node/polyfills/internal/child_process.ts index 56fc21f35..6f209b719 100644 --- a/ext/node/polyfills/internal/child_process.ts +++ b/ext/node/polyfills/internal/child_process.ts @@ -56,7 +56,12 @@ import { StringPrototypeSlice } from "ext:deno_node/internal/primordials.mjs"; import { StreamBase } from "ext:deno_node/internal_binding/stream_wrap.ts"; import { Pipe, socketType } from "ext:deno_node/internal_binding/pipe_wrap.ts"; import { Socket } from "node:net"; -import { kDetached, kExtraStdio, kIpc } from "ext:runtime/40_process.js"; +import { + kDetached, + kExtraStdio, + kIpc, + kNeedsNpmProcessState, +} from "ext:runtime/40_process.js"; export function mapValues<T, O>( record: Readonly<Record<string, T>>, @@ -281,6 +286,8 @@ export class ChildProcess extends EventEmitter { [kIpc]: ipc, // internal [kExtraStdio]: extraStdioNormalized, [kDetached]: detached, + // deno-lint-ignore no-explicit-any + [kNeedsNpmProcessState]: (options ?? {} as any)[kNeedsNpmProcessState], }).spawn(); this.pid = this.#process.pid; diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 6d24781cf..ea2978c49 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -118,6 +118,7 @@ rustyline = { workspace = true, features = ["custom-bindings"] } serde.workspace = true signal-hook = "0.3.17" signal-hook-registry = "1.4.0" +tempfile.workspace = true tokio.workspace = true tokio-metrics.workspace = true twox-hash.workspace = true diff --git a/runtime/js/40_process.js b/runtime/js/40_process.js index 2592c5c46..e2cb1d95b 100644 --- a/runtime/js/40_process.js +++ b/runtime/js/40_process.js @@ -160,6 +160,7 @@ function run({ export const kExtraStdio = Symbol("extraStdio"); export const kIpc = Symbol("ipc"); export const kDetached = Symbol("detached"); +export const kNeedsNpmProcessState = Symbol("needsNpmProcessState"); const illegalConstructorKey = Symbol("illegalConstructorKey"); @@ -178,6 +179,7 @@ function spawnChildInner(command, apiName, { [kDetached]: detached = false, [kExtraStdio]: extraStdio = [], [kIpc]: ipc = -1, + [kNeedsNpmProcessState]: needsNpmProcessState = false, } = { __proto__: null }) { const child = op_spawn_child({ cmd: pathFromURL(command), @@ -194,6 +196,7 @@ function spawnChildInner(command, apiName, { ipc, extraStdio, detached, + needsNpmProcessState, }, apiName); return new ChildProcess(illegalConstructorKey, { ...child, @@ -421,6 +424,7 @@ function spawnSync(command, { windowsRawArguments, extraStdio: [], detached: false, + needsNpmProcessState: false, }); return { success: result.status.success, diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 9134ac48a..0da2072b8 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -487,7 +487,6 @@ const NOT_IMPORTED_OPS = [ // to not depend on them. "op_set_exit_code", "op_napi_open", - "op_npm_process_state", ]; function removeImportedOps() { diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index a39bb5f04..530dcf49b 100644 --- a/runtime/ops/process.rs +++ b/runtime/ops/process.rs @@ -16,6 +16,7 @@ use deno_io::fs::FileResource; use deno_io::ChildStderrResource; use deno_io::ChildStdinResource; use deno_io::ChildStdoutResource; +use deno_io::IntoRawIoHandle; use deno_permissions::PermissionsContainer; use deno_permissions::RunQueryDescriptor; use serde::Deserialize; @@ -24,6 +25,7 @@ use std::borrow::Cow; use std::cell::RefCell; use std::collections::HashMap; use std::ffi::OsString; +use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::process::ExitStatus; @@ -117,6 +119,27 @@ impl StdioOrRid { } } +#[allow(clippy::disallowed_types)] +pub type NpmProcessStateProviderRc = + deno_fs::sync::MaybeArc<dyn NpmProcessStateProvider>; + +pub trait NpmProcessStateProvider: + std::fmt::Debug + deno_fs::sync::MaybeSend + deno_fs::sync::MaybeSync +{ + /// Gets a string containing the serialized npm state of the process. + /// + /// This will be set on the `DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE` environment + /// variable when doing a `child_process.fork`. The implementor can then check this environment + /// variable on startup to repopulate the internal npm state. + fn get_npm_process_state(&self) -> String { + // This method is only used in the CLI. + String::new() + } +} + +#[derive(Debug)] +pub struct EmptyNpmProcessStateProvider; +impl NpmProcessStateProvider for EmptyNpmProcessStateProvider {} deno_core::extension!( deno_process, ops = [ @@ -128,6 +151,10 @@ deno_core::extension!( deprecated::op_run_status, deprecated::op_kill, ], + options = { get_npm_process_state: Option<NpmProcessStateProviderRc> }, + state = |state, options| { + state.put::<NpmProcessStateProviderRc>(options.get_npm_process_state.unwrap_or(deno_fs::sync::MaybeArc::new(EmptyNpmProcessStateProvider))); + }, ); /// Second member stores the pid separately from the RefCell. It's needed for @@ -161,6 +188,7 @@ pub struct SpawnArgs { extra_stdio: Vec<Stdio>, detached: bool, + needs_npm_process_state: bool, } #[derive(Deserialize)] @@ -229,11 +257,64 @@ type CreateCommand = ( Vec<deno_io::RawBiPipeHandle>, ); +pub fn npm_process_state_tempfile( + contents: &[u8], +) -> Result<deno_io::RawIoHandle, AnyError> { + let mut temp_file = tempfile::tempfile()?; + temp_file.write_all(contents)?; + let handle = temp_file.into_raw_io_handle(); + #[cfg(windows)] + { + use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT; + // make the handle inheritable + // SAFETY: winapi call, handle is valid + unsafe { + windows_sys::Win32::Foundation::SetHandleInformation( + handle as _, + HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT, + ); + } + Ok(handle) + } + #[cfg(unix)] + { + // SAFETY: libc call, fd is valid + let inheritable = unsafe { + // duplicate the FD to get a new one that doesn't have the CLOEXEC flag set + // so it can be inherited by the child process + libc::dup(handle) + }; + // SAFETY: libc call, fd is valid + unsafe { + // close the old one + libc::close(handle); + } + Ok(inheritable) + } +} + +pub const NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME: &str = + "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE_FD"; + fn create_command( state: &mut OpState, mut args: SpawnArgs, api_name: &str, ) -> Result<CreateCommand, AnyError> { + let maybe_npm_process_state = if args.needs_npm_process_state { + let provider = state.borrow::<NpmProcessStateProviderRc>(); + let process_state = provider.get_npm_process_state(); + let fd = npm_process_state_tempfile(process_state.as_bytes())?; + args.env.push(( + NPM_RESOLUTION_STATE_FD_ENV_VAR_NAME.to_string(), + (fd as usize).to_string(), + )); + Some(fd) + } else { + None + }; + let (cmd, run_env) = compute_run_cmd_and_check_permissions( &args.cmd, args.cwd.as_deref(), @@ -301,6 +382,9 @@ fn create_command( let mut fds_to_dup = Vec::new(); let mut fds_to_close = Vec::new(); let mut ipc_rid = None; + if let Some(fd) = maybe_npm_process_state { + fds_to_close.push(fd); + } if let Some(ipc) = args.ipc { if ipc >= 0 { let (ipc_fd1, ipc_fd2) = deno_io::bi_pipe_pair_raw()?; @@ -369,6 +453,9 @@ fn create_command( { let mut ipc_rid = None; let mut handles_to_close = Vec::with_capacity(1); + if let Some(handle) = maybe_npm_process_state { + handles_to_close.push(handle); + } if let Some(ipc) = args.ipc { if ipc >= 0 { let (hd1, hd2) = deno_io::bi_pipe_pair_raw()?; @@ -506,23 +593,6 @@ fn spawn_child( }) } -fn close_raw_handle(handle: deno_io::RawBiPipeHandle) { - #[cfg(unix)] - { - // SAFETY: libc call - unsafe { - libc::close(handle); - } - } - #[cfg(windows)] - { - // SAFETY: win32 call - unsafe { - windows_sys::Win32::Foundation::CloseHandle(handle as _); - } - } -} - fn compute_run_cmd_and_check_permissions( arg_cmd: &str, arg_cwd: Option<&str>, @@ -690,7 +760,7 @@ fn op_spawn_child( create_command(state, args, &api_name)?; let child = spawn_child(state, command, pipe_rid, extra_pipe_rids, detached); for handle in handles_to_close { - close_raw_handle(handle); + deno_io::close_raw_handle(handle); } child } diff --git a/runtime/snapshot.rs b/runtime/snapshot.rs index db6688b46..f20c04f08 100644 --- a/runtime/snapshot.rs +++ b/runtime/snapshot.rs @@ -302,7 +302,7 @@ pub fn create_runtime_snapshot( ops::permissions::deno_permissions::init_ops(Arc::new( RuntimePermissionDescriptorParser::new(fs), )), - ops::process::deno_process::init_ops(), + ops::process::deno_process::init_ops(None), ops::signal::deno_signal::init_ops(), ops::tty::deno_tty::init_ops(), ops::http::deno_http_runtime::init_ops(), diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index 7b69a9798..8892d5bc6 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::inspector_server::InspectorServer; use crate::ops; +use crate::ops::process::NpmProcessStateProviderRc; use crate::ops::worker_host::WorkersTable; use crate::shared::maybe_transpile_source; use crate::shared::runtime; @@ -383,6 +384,7 @@ pub struct WebWorkerOptions { pub strace_ops: Option<Vec<String>>, pub close_on_idle: bool, pub maybe_worker_metadata: Option<WorkerMetadata>, + pub npm_process_state_provider: Option<NpmProcessStateProviderRc>, } impl WebWorker { @@ -507,7 +509,9 @@ impl WebWorker { ops::permissions::deno_permissions::init_ops_and_esm( options.permission_desc_parser.clone(), ), - ops::process::deno_process::init_ops_and_esm(), + ops::process::deno_process::init_ops_and_esm( + options.npm_process_state_provider, + ), ops::signal::deno_signal::init_ops_and_esm(), ops::tty::deno_tty::init_ops_and_esm(), ops::http::deno_http_runtime::init_ops_and_esm(), diff --git a/runtime/worker.rs b/runtime/worker.rs index 3d8c8a0b9..f72e6d7c0 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -49,6 +49,7 @@ use crate::code_cache::CodeCache; use crate::code_cache::CodeCacheType; use crate::inspector_server::InspectorServer; use crate::ops; +use crate::ops::process::NpmProcessStateProviderRc; use crate::permissions::RuntimePermissionDescriptorParser; use crate::shared::maybe_transpile_source; use crate::shared::runtime; @@ -158,6 +159,7 @@ pub struct WorkerOptions { /// executed tries to load modules. pub module_loader: Rc<dyn ModuleLoader>, pub node_services: Option<NodeExtInitServices>, + pub npm_process_state_provider: Option<NpmProcessStateProviderRc>, pub permission_desc_parser: Arc<dyn deno_permissions::PermissionDescriptorParser>, // Callbacks invoked when creating new instance of WebWorker @@ -235,6 +237,7 @@ impl Default for WorkerOptions { extensions: Default::default(), startup_snapshot: Default::default(), create_params: Default::default(), + npm_process_state_provider: Default::default(), bootstrap: Default::default(), stdio: Default::default(), feature_checker: Default::default(), @@ -437,7 +440,9 @@ impl MainWorker { ops::permissions::deno_permissions::init_ops_and_esm( options.permission_desc_parser, ), - ops::process::deno_process::init_ops_and_esm(), + ops::process::deno_process::init_ops_and_esm( + options.npm_process_state_provider, + ), ops::signal::deno_signal::init_ops_and_esm(), ops::tty::deno_tty::init_ops_and_esm(), ops::http::deno_http_runtime::init_ops_and_esm(), diff --git a/tests/registry/npm/@denotest/child-process-fork/1.0.0/index.js b/tests/registry/npm/@denotest/child-process-fork/1.0.0/index.js index 0482be404..ca0ba246a 100644 --- a/tests/registry/npm/@denotest/child-process-fork/1.0.0/index.js +++ b/tests/registry/npm/@denotest/child-process-fork/1.0.0/index.js @@ -1,14 +1,9 @@ const path = require("path"); +const childProcess = require("node:child_process"); function childProcessFork(path) { - const command = new Deno.Command(Deno.execPath(), { - args: ["run", "-A", path], - env: { - "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE": Deno[Deno.internal].core.ops.op_npm_process_state(), - } - }); - const child = command.spawn(); - child.status.then(() => { + const child = childProcess.fork(path); + child.on("exit", () => { console.log("Done."); }); } diff --git a/tests/unit/ops_test.ts b/tests/unit/ops_test.ts index 6de55f8b6..4ba7c5ce3 100644 --- a/tests/unit/ops_test.ts +++ b/tests/unit/ops_test.ts @@ -1,6 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -const EXPECTED_OP_COUNT = 12; +const EXPECTED_OP_COUNT = 11; Deno.test(function checkExposedOps() { // @ts-ignore TS doesn't allow to index with symbol |