Skip to content

Commit

Permalink
fix(node): child_process IPC on Windows (#21597)
Browse files Browse the repository at this point in the history
This PR implements the child_process IPC pipe between parent and child.
The implementation uses Windows named pipes created by parent and passes
the inheritable file handle to the child.

I've also replace parts of the initial implementation which passed the
raw parent fd to JS with resource ids instead. This way no file handle
is exposed to the JS land (both parent and child).

`IpcJsonStreamResource` can stream upto 800MB/s of JSON data on Win 11
AMD Ryzen 7 16GB (without `memchr` vectorization)
  • Loading branch information
littledivy committed Dec 19, 2023
1 parent aefa205 commit 55fac9f
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 93 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions cli/args/mod.rs
Expand Up @@ -917,12 +917,12 @@ impl CliOptions {
.map(Some)
}

pub fn node_ipc_fd(&self) -> Option<i32> {
pub fn node_ipc_fd(&self) -> Option<i64> {
let maybe_node_channel_fd = std::env::var("DENO_CHANNEL_FD").ok();
if let Some(node_channel_fd) = maybe_node_channel_fd {
// Remove so that child processes don't inherit this environment variable.
std::env::remove_var("DENO_CHANNEL_FD");
node_channel_fd.parse::<i32>().ok()
node_channel_fd.parse::<i64>().ok()
} else {
None
}
Expand Down
4 changes: 2 additions & 2 deletions cli/worker.rs
Expand Up @@ -124,7 +124,7 @@ struct SharedWorkerState {
maybe_inspector_server: Option<Arc<InspectorServer>>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
feature_checker: Arc<FeatureChecker>,
node_ipc: Option<i32>,
node_ipc: Option<i64>,
}

impl SharedWorkerState {
Expand Down Expand Up @@ -404,7 +404,7 @@ impl CliMainWorkerFactory {
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
feature_checker: Arc<FeatureChecker>,
options: CliMainWorkerOptions,
node_ipc: Option<i32>,
node_ipc: Option<i64>,
) -> Self {
Self {
shared: Arc::new(SharedWorkerState {
Expand Down
4 changes: 4 additions & 0 deletions ext/node/Cargo.toml
Expand Up @@ -74,3 +74,7 @@ url.workspace = true
winapi.workspace = true
x25519-dalek = "2.0.0"
x509-parser = "0.15.0"

[target.'cfg(windows)'.dependencies]
windows-sys.workspace = true
winapi = { workspace = true, features = ["consoleapi"] }
2 changes: 1 addition & 1 deletion ext/node/lib.rs
Expand Up @@ -31,6 +31,7 @@ mod polyfill;
mod resolution;

pub use ops::ipc::ChildPipeFd;
pub use ops::ipc::IpcJsonStreamResource;
pub use ops::v8::VM_CONTEXT_INDEX;
pub use package_json::PackageJson;
pub use path::PathClean;
Expand Down Expand Up @@ -307,7 +308,6 @@ deno_core::extension!(deno_node,
ops::require::op_require_break_on_next_statement,
ops::util::op_node_guess_handle_type,
ops::crypto::op_node_create_private_key,
ops::ipc::op_node_ipc_pipe,
ops::ipc::op_node_child_ipc_pipe,
ops::ipc::op_node_ipc_write,
ops::ipc::op_node_ipc_read,
Expand Down

0 comments on commit 55fac9f

Please sign in to comment.