Skip to content

feat: add ProcessSignaler to expose child process PIDs#160

Draft
wolfv wants to merge 4 commits intodenoland:mainfrom
wolfv:process-signaler
Draft

feat: add ProcessSignaler to expose child process PIDs#160
wolfv wants to merge 4 commits intodenoland:mainfrom
wolfv:process-signaler

Conversation

@wolfv
Copy link
Contributor

@wolfv wolfv commented Nov 28, 2025

Add ProcessSignaler type to track spawned child process PIDs, enabling
proper signal forwarding by allowing callers to check process groups
before forwarding signals.

  • Add ProcessSignaler with current_pid() and subscribe() methods
  • Add execute_with_signaler() convenience function
  • Add ShellState::new_with_process_signaler() constructor
  • Notify on process spawn/exit in ExecutableCommand

@dsherret
Copy link
Member

Add ProcessSignaler type to track spawned child process PIDs, enabling
proper signal forwarding by allowing callers to check process groups
before forwarding signals.

I'm a little confused about this PR. Is this a replacement for KillSignal? Can you explain it a little more and show a use case? I'm not that familiar with process groups and when a spawned process would end up in a different process group (how could that happen?). Thanks!

/// Returns the PID of the current foreground child process, if any.
///
/// Returns `None` if no child process is currently running.
pub fn current_pid(&self) -> Option<u32> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be multiple child processes running at the same time:

deno eval 'console.log(1)' ; deno eval 'console.log(2)' &

It seems this would only hold one of them?

@wolfv
Copy link
Contributor Author

wolfv commented Nov 28, 2025

Hey @dsherret to be completely transparent the changes here were written by Claude :) I didn't expect you to already take a look tbh.

I am trying to fix an issue in Pixi where a user has issues to programmatically send CTRL+C to the process.

ProcessSignaler is complementary to KillSignal, not a replacement.

  • KillSignal is for sending signals to child processes (pixi → child)
  • ProcessSignaler is for observing child process PIDs so the caller can make decisions about signal forwarding

The Problem: When you run pixi run python script.py in a terminal and press CTRL+C:

The terminal sends SIGINT to the entire foreground process group

  • Both pixi AND the python child receive SIGINT directly from the terminal
  • But when you run kill -INT <pixi_pid> from another terminal or a job scheduler (like SLURM):
    • Only pixi receives SIGINT
    • The child never gets the signal unless pixi forwards it

The challenge is: pixi needs to forward signals sent via kill, but should avoid double-forwarding when CTRL+C already delivered the signal to the child.

When would a child be in a different process group?

By default, child processes inherit the parent's process group. But processes can call setpgid() or setsid() to create a new process group. This happens with:

  • Daemons/background services
  • Some shell commands that manage their own process groups
  • Programs using job control

@wolfv
Copy link
Contributor Author

wolfv commented Nov 28, 2025

Here is an explanation in uv about the same issue: https://github.com/astral-sh/uv/blob/5f3d46c2413225aac68c86fa24be97c4c2c193e4/crates/uv/src/child.rs#L12-L24

How are you solving this in deno? :)

@dsherret
Copy link
Member

Thanks! It seems a bit complicated to introduce a separate concept for this. Is there a way we could build this into KillSignal instead? For example, a way to say "send this signal only if a different process group".

Comment on lines +687 to +690
/// if child_pgid != our_pgid {
/// // Child in different process group, forward signal
/// kill_signal.send(SignalKind::SIGINT);
/// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would work because there could be multiple child processes running in different groups?

@wolfv wolfv marked this pull request as draft November 29, 2025 08:52
@wolfv
Copy link
Contributor Author

wolfv commented Nov 29, 2025

I still need to test in pixi whether this fixes our CTRL+C / kill signal issues. Marked the PR as draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants