Skip to content

Commit 90c7921

Browse files
hyperpolymathclaude
andcommitted
feat: Phase 0 Sealing - SIGINT handling for external commands
Implemented graceful Ctrl+C handling that interrupts running external commands without killing the shell: Changes: - Added ctrlc = "3.4" dependency for cross-platform signal handling - Added INTERRUPT_REQUESTED global AtomicBool flag in main.rs - Installed SIGINT handler in repl::run() to set interrupt flag - Updated execute_external() to use .spawn() + polling loop - Updated execute_external_with_redirects() with same pattern - Added process group management on Unix for proper job control Behavior: - External commands now check interrupt flag every 50ms - Ctrl+C kills child process and returns exit code 130 (128 + SIGINT) - Shell continues running after interrupt - No zombie processes left behind Files changed: - Cargo.toml: Added ctrlc dependency - src/main.rs: Added INTERRUPT_REQUESTED static - src/external.rs: Changed .status() → .spawn() + try_wait() loop - src/repl.rs: Installed signal handler at REPL start - tests/manual_sigint_test.sh: Manual verification script Test results: 90/90 passing (no regressions) Phase 0 Sealing: Component 1/6 complete Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 551e0f7 commit 90c7921

5 files changed

Lines changed: 166 additions & 17 deletions

File tree

impl/rust-cli/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ chrono = { version = "0.4", features = ["serde"] }
4545
# Stdout/stderr capture for redirections
4646
gag = "1.0"
4747

48+
# Signal handling (Ctrl+C for external commands)
49+
ctrlc = "3.4"
50+
4851
# Unix socket for BEAM IPC
4952
# (tokio already provides this)
5053

impl/rust-cli/src/external.rs

Lines changed: 79 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ use anyhow::{Context, Result};
88
use std::fs::{File, OpenOptions};
99
use std::path::PathBuf;
1010
use std::process::{Command, Stdio};
11+
use std::time::Duration;
12+
use std::sync::atomic::Ordering;
1113

1214
use crate::redirection::{Redirection, RedirectSetup};
1315
use crate::state::ShellState;
@@ -36,40 +38,100 @@ pub fn execute_external_with_redirects(
3638
// PATH lookup
3739
let executable = find_in_path(program)?;
3840

39-
// Execute with redirected stdio
40-
let status = Command::new(&executable)
41+
// Build command with redirected stdio
42+
let mut command = Command::new(&executable);
43+
command
4144
.args(args)
4245
.stdin(stdin_cfg)
4346
.stdout(stdout_cfg)
44-
.stderr(stderr_cfg)
45-
.status()
46-
.context(format!("Failed to execute: {}", program))?;
47+
.stderr(stderr_cfg);
4748

48-
// Record file modifications for undo
49-
redirect_setup.record_for_undo(state)?;
49+
// Set process group on Unix for proper job control
50+
#[cfg(unix)]
51+
{
52+
use std::os::unix::process::CommandExt;
53+
command.process_group(0);
54+
}
5055

51-
// Handle exit status (including signals)
52-
handle_exit_status(status)
56+
// Spawn child process (non-blocking)
57+
let mut child = command
58+
.spawn()
59+
.context(format!("Failed to spawn: {}", program))?;
60+
61+
// Poll for exit while checking for interrupt
62+
loop {
63+
match child.try_wait().context("Failed to wait for child")? {
64+
Some(status) => {
65+
// Child exited - record modifications and return
66+
redirect_setup.record_for_undo(state)?;
67+
return handle_exit_status(status);
68+
}
69+
None => {
70+
// Child still running - check for interrupt
71+
if crate::INTERRUPT_REQUESTED.load(Ordering::Relaxed) {
72+
// User pressed Ctrl+C - kill child and reset flag
73+
child.kill().context("Failed to kill child process")?;
74+
crate::INTERRUPT_REQUESTED.store(false, Ordering::Relaxed);
75+
76+
// Wait for child to actually terminate
77+
child.wait().context("Failed to wait for killed child")?;
78+
79+
// Don't record modifications - operation interrupted
80+
return Ok(130); // 128 + SIGINT(2)
81+
}
82+
83+
// Sleep briefly before next check
84+
std::thread::sleep(Duration::from_millis(50));
85+
}
86+
}
87+
}
5388
}
5489

5590
/// Execute an external command (no redirections - legacy)
5691
///
5792
/// Handles signal termination (SIGINT, SIGTERM) and returns appropriate exit codes.
5893
/// Exit code 130 indicates SIGINT (Ctrl+C), 143 indicates SIGTERM.
94+
///
95+
/// This function supports interruption via Ctrl+C - the global INTERRUPT_REQUESTED
96+
/// flag is checked periodically and the child process is killed if set.
5997
pub fn execute_external(program: &str, args: &[String]) -> Result<i32> {
6098
// PATH lookup
6199
let executable = find_in_path(program)?;
62100

63-
// Execute via std::process::Command
64-
let status = Command::new(&executable)
101+
// Spawn child process (non-blocking)
102+
let mut child = Command::new(&executable)
65103
.args(args)
66-
.stdin(Stdio::inherit()) // Pass through stdin
67-
.stdout(Stdio::inherit()) // Pass through stdout
68-
.stderr(Stdio::inherit()) // Pass through stderr
69-
.status()
70-
.context(format!("Failed to execute: {}", program))?;
104+
.stdin(Stdio::inherit())
105+
.stdout(Stdio::inherit())
106+
.stderr(Stdio::inherit())
107+
.spawn()
108+
.context(format!("Failed to spawn: {}", program))?;
109+
110+
// Poll for exit while checking for interrupt
111+
loop {
112+
match child.try_wait().context("Failed to wait for child")? {
113+
Some(status) => {
114+
// Child exited normally
115+
return handle_exit_status(status);
116+
}
117+
None => {
118+
// Child still running - check for interrupt
119+
if crate::INTERRUPT_REQUESTED.load(Ordering::Relaxed) {
120+
// User pressed Ctrl+C - kill child and reset flag
121+
child.kill().context("Failed to kill child process")?;
122+
crate::INTERRUPT_REQUESTED.store(false, Ordering::Relaxed);
123+
124+
// Wait for child to actually terminate
125+
child.wait().context("Failed to wait for killed child")?;
126+
127+
return Ok(130); // 128 + SIGINT(2)
128+
}
71129

72-
handle_exit_status(status)
130+
// Sleep briefly before next check
131+
std::thread::sleep(Duration::from_millis(50));
132+
}
133+
}
134+
}
73135
}
74136

75137
/// Convert redirections to stdio configuration

impl/rust-cli/src/main.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ mod state;
2525
use anyhow::{Context, Result};
2626
use clap::{Parser, Subcommand};
2727
use colored::Colorize;
28+
use std::sync::atomic::{AtomicBool, Ordering};
29+
30+
/// Global flag set by SIGINT handler to interrupt running commands
31+
pub static INTERRUPT_REQUESTED: AtomicBool = AtomicBool::new(false);
2832

2933
/// Valence Shell - Every operation is reversible
3034
#[derive(Parser)]

impl/rust-cli/src/repl.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,20 @@
99
use anyhow::Result;
1010
use colored::Colorize;
1111
use std::io::{self, BufRead, Write};
12+
use std::sync::atomic::Ordering;
1213

1314
use crate::executable::{ExecutableCommand, ExecutionResult};
1415
use crate::parser;
1516
use crate::state::ShellState;
1617

1718
/// Run the interactive REPL
1819
pub fn run(state: &mut ShellState) -> Result<()> {
20+
// Install SIGINT handler for graceful Ctrl+C handling
21+
ctrlc::set_handler(move || {
22+
crate::INTERRUPT_REQUESTED.store(true, Ordering::Relaxed);
23+
})
24+
.expect("Error setting Ctrl-C handler");
25+
1926
let stdin = io::stdin();
2027
let mut stdout = io::stdout();
2128

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#!/bin/bash
2+
# SPDX-License-Identifier: PLMP-1.0-or-later
3+
# Manual test for SIGINT (Ctrl+C) handling
4+
#
5+
# Expected behavior:
6+
# 1. Shell starts
7+
# 2. sleep 5 command runs
8+
# 3. Ctrl+C interrupts sleep (not shell)
9+
# 4. Shell remains responsive
10+
# 5. No zombie processes
11+
12+
set -e
13+
14+
echo "=== SIGINT Handling Manual Test ==="
15+
echo
16+
echo "This test will:"
17+
echo "1. Start vsh in interactive mode"
18+
echo "2. Run 'sleep 10' command"
19+
echo "3. Send SIGINT (Ctrl+C) after 2 seconds"
20+
echo "4. Verify shell is still alive"
21+
echo "5. Check for zombie processes"
22+
echo
23+
echo "Starting test in 3 seconds..."
24+
sleep 3
25+
26+
# Build release binary
27+
echo "Building vsh..."
28+
cargo build --release --quiet
29+
30+
# Create test script
31+
TEST_SCRIPT=$(mktemp)
32+
cat > "$TEST_SCRIPT" <<'EOF'
33+
sleep 10
34+
echo "Shell is still alive after interrupt"
35+
exit
36+
EOF
37+
38+
# Run vsh with test script, send SIGINT after 2 seconds
39+
echo
40+
echo "Running vsh with 'sleep 10' command..."
41+
echo "Will send Ctrl+C after 2 seconds..."
42+
echo
43+
44+
(
45+
sleep 2
46+
pkill -INT -f "sleep 10" || true
47+
echo " [Sent SIGINT]"
48+
) &
49+
50+
timeout 15 ./target/release/vsh < "$TEST_SCRIPT" || true
51+
52+
# Check for zombie processes
53+
echo
54+
echo "Checking for zombie processes..."
55+
ZOMBIES=$(ps aux | grep -E "defunct|Z" | grep -v grep || true)
56+
57+
if [ -n "$ZOMBIES" ]; then
58+
echo "❌ FAIL: Found zombie processes:"
59+
echo "$ZOMBIES"
60+
exit 1
61+
else
62+
echo "✅ PASS: No zombie processes"
63+
fi
64+
65+
# Cleanup
66+
rm -f "$TEST_SCRIPT"
67+
68+
echo
69+
echo "=== Test Complete ==="
70+
echo "Manual verification:"
71+
echo "1. Sleep command should have been interrupted"
72+
echo "2. Shell should have printed 'Shell is still alive after interrupt'"
73+
echo "3. No zombie processes found"

0 commit comments

Comments
 (0)