kill all jobs when exiting an interactive shell

Fish is not consistent with other shells like bash and zsh when exiting
an interactive shell with background jobs. While it is true that fish
explicitly claims no compatibility with POSIX 1003.1 this is an area
where deviation from the established practice adds negative value.

The reason for the current behavior seems to be due to two users who did
not understand why interactive shells managed background jobs as they
did and were not aware of tools like `nohup` or `disown`. See issue

There is also a fairly significant bug present due to a misunderstanding of
what a true value from `reader_exit_forced()` means. This change corrects
that misunderstanding.

Fixes #3497
This commit is contained in:
Kurtis Rader 2016-12-17 20:21:27 -08:00
parent 9f8d854c2a
commit 8d27f81a7b

View file

@ -2222,48 +2222,40 @@ bool shell_is_exiting() {
/// This function is called when the main loop notices that end_loop has been set while in /// This function is called when the main loop notices that end_loop has been set while in
/// interactive mode. It checks if it is ok to exit. /// interactive mode. It checks if it is ok to exit.
static void handle_end_loop() { static void handle_end_loop() {
job_t *j; bool bg_jobs = false;
int stopped_jobs_count = 0; bool is_breakpoint = false;
int is_breakpoint = 0;
const parser_t &parser = parser_t::principal_parser(); const parser_t &parser = parser_t::principal_parser();
for (size_t i = 0; i < parser.block_count(); i++) { for (size_t i = 0; i < parser.block_count(); i++) {
if (parser.block_at_index(i)->type() == BREAKPOINT) { if (parser.block_at_index(i)->type() == BREAKPOINT) {
is_breakpoint = 1; is_breakpoint = true;
break; break;
} }
} }
job_iterator_t jobs; job_iterator_t jobs;
while ((j = jobs.next())) { while (job_t *j = jobs.next()) {
if (!job_is_completed(j) && (job_is_stopped(j))) { if (!job_is_completed(j)) {
stopped_jobs_count++; bg_jobs = true;
break; break;
} }
} }
if (!reader_exit_forced() && !data->prev_end_loop && stopped_jobs_count && !is_breakpoint) { if (!reader_exit_forced() && !data->prev_end_loop && bg_jobs && !is_breakpoint) {
writestr(_( writestr(_(L"There are stopped or running jobs.\n"));
L"There are stopped jobs. A second attempt to exit will enforce their termination.\n")); writestr(_(L"A second attempt to exit will force their termination.\n"));
reader_exit(0, 0); reader_exit(0, 0);
data->prev_end_loop = 1; data->prev_end_loop = 1;
} else { } else {
// PCA: We used to only hangup jobs if stdin was closed. This prevented child processes from // Kill background jobs.
// exiting. It's unclear to my why it matters if stdin is closed, but it seems to me if
// we're forcing an exit, we definitely want to hang up our processes. See issue #138.
if (reader_exit_forced() || !isatty(0)) {
// We already know that stdin is a tty since we're in interactive mode. If isatty
// returns false, it means stdin must have been closed.
job_iterator_t jobs; job_iterator_t jobs;
while ((j = jobs.next())) { while (job_t *j = jobs.next()) {
// Send SIGHUP only to foreground processes. See issue #1771. if (!job_is_completed(j)) {
if (!job_is_completed(j) && job_get_flag(j, JOB_FOREGROUND)) { if (job_is_stopped(j)) job_signal(j, SIGCONT);
job_signal(j, SIGHUP); job_signal(j, SIGHUP);
} }
} }
} }
}
} }
static bool selection_is_at_top() { static bool selection_is_at_top() {