-
Notifications
You must be signed in to change notification settings - Fork 6k
fix(ext/node): implement timeout and pid for child_process spawnSync #32810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
94b1424
f10d042
810b45d
e77a900
7761968
b5cb14e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -250,6 +250,11 @@ | |||||||||||
| extra_stdio: Vec<Stdio>, | ||||||||||||
| detached: bool, | ||||||||||||
| needs_npm_process_state: bool, | ||||||||||||
|
|
||||||||||||
| #[serde(default)] | ||||||||||||
| timeout: Option<u64>, | ||||||||||||
| #[serde(default)] | ||||||||||||
| kill_signal: Option<String>, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #[derive(Deserialize)] | ||||||||||||
|
|
@@ -393,9 +398,11 @@ | |||||||||||
|
|
||||||||||||
| #[derive(ToV8)] | ||||||||||||
| pub struct SpawnOutput { | ||||||||||||
| pid: u32, | ||||||||||||
| status: ChildStatus, | ||||||||||||
| stdout: Option<Uint8Array>, | ||||||||||||
| stderr: Option<Uint8Array>, | ||||||||||||
| killed_by_timeout: bool, | ||||||||||||
bartlomieju marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type CreateCommand = ( | ||||||||||||
|
|
@@ -1110,28 +1117,75 @@ | |||||||||||
| let stdout = matches!(args.stdio.stdout, StdioOrRid::Stdio(Stdio::Piped)); | ||||||||||||
| let stderr = matches!(args.stdio.stderr, StdioOrRid::Stdio(Stdio::Piped)); | ||||||||||||
| let input = args.input.clone(); | ||||||||||||
| let timeout = args.timeout; | ||||||||||||
| let kill_signal_str = args.kill_signal.clone(); | ||||||||||||
| let (mut command, _, _, _) = | ||||||||||||
| create_command(state, args, "Deno.Command().outputSync()")?; | ||||||||||||
|
|
||||||||||||
| let mut child = command.spawn().map_err(|e| ProcessError::SpawnFailed { | ||||||||||||
| command: command.get_program().to_string_lossy().into_owned(), | ||||||||||||
| error: Box::new(e.into()), | ||||||||||||
| })?; | ||||||||||||
| let pid = child.id(); | ||||||||||||
| if let Some(input) = input { | ||||||||||||
| let mut stdin = child.stdin.take().ok_or_else(|| { | ||||||||||||
| ProcessError::Io(std::io::Error::other("stdin is not available")) | ||||||||||||
| })?; | ||||||||||||
| stdin.write_all(&input)?; | ||||||||||||
| stdin.flush()?; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // If timeout is specified, spawn a thread that will kill the child | ||||||||||||
| // after the timeout expires. | ||||||||||||
| let killed_by_timeout = | ||||||||||||
| std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); | ||||||||||||
| if let Some(timeout_ms) = timeout { | ||||||||||||
| if timeout_ms > 0 { | ||||||||||||
| let child_id = child.id(); | ||||||||||||
| let killed = killed_by_timeout.clone(); | ||||||||||||
| let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); | ||||||||||||
| #[cfg(unix)] | ||||||||||||
| let signal = | ||||||||||||
| deno_signals::signal_str_to_int(kill_signal).unwrap_or(libc::SIGTERM); | ||||||||||||
|
||||||||||||
| let signal = | |
| deno_signals::signal_str_to_int(kill_signal).unwrap_or(libc::SIGTERM); | |
| let signal = deno_signals::signal_str_to_int(kill_signal) | |
| .or_else(|| kill_signal.parse::<i32>().ok()) | |
| .unwrap_or(libc::SIGTERM); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that
spawnSyncforwardstimeout/killSignalintoDeno.Commandand interprets_killedByTimeout, theSpawnSyncOptions.killSignaltyping/comment earlier in this file is out of date (it currently says the option is not implemented and only types it asstring). Consider updating the type tostring | number(Node accepts both) and removing/updating the "not yet implemented" note to match the new behavior.