Skip to content

Commit 89d7cbd

Browse files
Parth SharmaParth Sharma
authored andcommitted
Let a finished background session's jobs start before hanging it up
The shell passes its SIGHUP on to every job, and nohup ignores SIGHUP only once it runs. A background session whose last command was 'nohup server &' was hung up the moment its end marker arrived, so the SIGHUP reached the job while it was still being forked and killed it. hang_up now waits 200 ms first when the shell has jobs (stop::let_jobs_start), so nohup and disowned jobs survive; plain jobs still die with the shell. kill/drop are unchanged.
1 parent fe7426c commit 89d7cbd

3 files changed

Lines changed: 21 additions & 6 deletions

File tree

‎crates/terminal/src/lib.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,8 @@ impl Session {
305305

306306
/// Stops the shell and the command it is running. Background jobs get
307307
/// the shell's SIGHUP, as when a terminal window closes, so jobs started
308-
/// with `nohup` (e.g. a server) keep running.
308+
/// with `nohup` (e.g. a server) keep running, including one started just
309+
/// before the hang-up.
309310
pub(crate) fn hang_up(&mut self) {
310311
self.end_shell(Ending::Foreground);
311312
}
@@ -315,12 +316,13 @@ impl Session {
315316
if let Some(pid) = shell.child.process_id().and_then(|p| i32::try_from(p).ok()) {
316317
let pid = Pid::from_raw(pid);
317318
let foreground = shell.master.process_group_leader().map(Pid::from_raw);
319+
let tty = shell.master.tty_name();
318320
match ending {
319-
Ending::Session => {
320-
let tty = shell.master.tty_name();
321-
stop::stop_session(pid, tty.as_deref(), foreground);
321+
Ending::Session => stop::stop_session(pid, tty.as_deref(), foreground),
322+
Ending::Foreground => {
323+
stop::stop_foreground(pid, foreground);
324+
stop::let_jobs_start(pid, tty.as_deref());
322325
}
323-
Ending::Foreground => stop::stop_foreground(pid, foreground),
324326
}
325327
}
326328
// SIGHUP, then SIGKILL, to the shell. Errors only mean it already

‎crates/terminal/src/stop.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ use nix::unistd::Pid;
1616
const TERM_GRACE: Duration = Duration::from_secs(1);
1717
const TERM_POLL: Duration = Duration::from_millis(20);
1818

19+
/// How long the shell's jobs get to start before it is hung up. The shell
20+
/// passes its SIGHUP on to every job, and `nohup` ignores SIGHUP only once it
21+
/// runs, so a job forked just before the hang-up (`nohup server &` as the
22+
/// last command) would die with the shell.
23+
const JOB_START_GRACE: Duration = Duration::from_millis(200);
24+
1925
/// Sends SIGTERM to every process group in the shell's session, waits up to
2026
/// [`TERM_GRACE`] for them to exit, then sends SIGKILL to whatever is left,
2127
/// the shell included. `tty` is the shell's terminal; `foreground` is that
@@ -47,6 +53,13 @@ pub(crate) fn stop_foreground(shell: Pid, foreground: Option<Pid>) {
4753
signal_groups(&[group], Signal::SIGKILL);
4854
}
4955

56+
/// Waits [`JOB_START_GRACE`] if the shell has jobs, before it is hung up.
57+
pub(crate) fn let_jobs_start(shell: Pid, tty: Option<&Path>) {
58+
if session_groups(shell, tty).iter().any(|&group| group != shell) {
59+
std::thread::sleep(JOB_START_GRACE);
60+
}
61+
}
62+
5063
/// Sends SIGTERM to `groups` and waits up to [`TERM_GRACE`] for all of them
5164
/// but the shell's own to exit (an interactive shell ignores SIGTERM).
5265
fn terminate(groups: &[Pid], shell: Pid) {

‎docs/HOW-IT-WORKS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ A command is not typed into the shell. It is written to a script file, and the s
4141

4242
The end marker carries the exit status and the working directory, and because the command itself is sourced from a file, quotes, heredocs and commands that read stdin cannot swallow the marker. Working directory, environment and background jobs carry from one turn to the next, like a person's terminal.
4343

44-
A command that has not finished after `exec_default_yield_ms` (10 seconds by default, `runtime.timeout` in agent.yaml) is left running in the background: the model gets the output so far and a session id, and can wait for more, send input or interrupt it with `write_stdin`. Up to `MAX_SESSIONS` shells exist at once. With `--linger` the shells stay open after the run, so a server the agent started keeps serving whoever needs it next. Otherwise, when the run ends, times out or is cancelled (an IPC `abort`, `kill`, closed stdin, or SIGTERM/SIGINT to `gitagent --ipc`), every process group in each shell's session gets SIGTERM and then, after at most a second, SIGKILL, so background jobs and the running command stop with the shell. A background session whose command finished only hangs up its shell, so a server it started with `nohup` keeps running.
44+
A command that has not finished after `exec_default_yield_ms` (10 seconds by default, `runtime.timeout` in agent.yaml) is left running in the background: the model gets the output so far and a session id, and can wait for more, send input or interrupt it with `write_stdin`. Up to `MAX_SESSIONS` shells exist at once. With `--linger` the shells stay open after the run, so a server the agent started keeps serving whoever needs it next. Otherwise, when the run ends, times out or is cancelled (an IPC `abort`, `kill`, closed stdin, or SIGTERM/SIGINT to `gitagent --ipc`), every process group in each shell's session gets SIGTERM and then, after at most a second, SIGKILL, so background jobs and the running command stop with the shell. A background session whose command finished only hangs up its shell, so a server it started with `nohup` keeps running. If that shell has jobs, the hang-up waits 0.2 s first: the shell passes its SIGHUP on to every job, and `nohup` only ignores it once it runs, so `nohup server &` as the last command would otherwise die with the shell.
4545

4646
## 4. The output shaper
4747

0 commit comments

Comments
 (0)