diff --git a/src/common.cpp b/src/common.cpp index 8e4285f0e..ea103fd08 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -109,7 +109,6 @@ demangled_backtrace(int max_frames, int skip_levels) { void __attribute__((noinline)) show_stackframe(const wchar_t msg_level, int frame_count, int skip_levels) { - ASSERT_IS_NOT_FORKED_CHILD(); if (frame_count < 1) return; // TODO: Decide if this is still needed. I'm commenting it out because it caused me some grief diff --git a/src/exec.cpp b/src/exec.cpp index 8b4d57528..3b6037a36 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -166,7 +166,6 @@ static void safe_launch_process(process_t *p, const char *actual_cmd, const char char *const *argv = const_cast(cargv); execve(actual_cmd, argv, envv); - err = errno; // Something went wrong with execve, check for a ":", and run /bin/sh if encountered. This is a diff --git a/src/fish.cpp b/src/fish.cpp index 3df48c738..572fbe3e0 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -368,7 +368,9 @@ int main(int argc, char **argv) { // TODO: Remove this once we're confident that not blocking/unblocking every signal around // some critical sections is no longer necessary. env_var_t fish_no_signal_block = env_get_string(L"FISH_NO_SIGNAL_BLOCK"); - if (!fish_no_signal_block.missing()) ignore_signal_block = true; + if (!fish_no_signal_block.missing_or_empty() && !from_string(fish_no_signal_block)) { + ignore_signal_block = false; + } // Stomp the exit status of any initialization commands (issue #635). proc_set_last_status(STATUS_BUILTIN_OK); diff --git a/src/postfork.cpp b/src/postfork.cpp index 8b6c0c24a..6eb0f38de 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -108,7 +108,9 @@ bool set_child_group(job_t *j, process_t *p, int print_errors) { int result = -1; errno = EINTR; while (result == -1 && errno == EINTR) { + signal_block(true); result = tcsetpgrp(STDIN_FILENO, j->pgid); + signal_unblock(true); } if (result == -1) { if (errno == ENOTTY) redirect_tty_output(); diff --git a/src/proc.cpp b/src/proc.cpp index 2a57e5ab1..7b070ce73 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -783,6 +783,14 @@ static void read_try(job_t *j) { /// \param cont If this variable is set, we are giving back control to a job that has previously /// been stopped. In that case, we need to set the terminal attributes to those saved in the job. static bool terminal_give_to_job(job_t *j, int cont) { + int s = 0; + errno = 0; + if (j->pgid == 0) { + debug(2, "terminal_give_to_job() returning early due to no process group"); + return true; + } + + signal_block(true); int result = -1; errno = EINTR; while (result == -1 && errno == EINTR) { @@ -792,11 +800,15 @@ static bool terminal_give_to_job(job_t *j, int cont) { if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr()); wperror(L"tcsetpgrp"); + signal_unblock(true); return false; } if (cont) { int result = -1; + // TODO: Remove this EINTR loop since we have blocked all signals and thus cannot be + // interrupted. I'm leaving it in place because all of the logic involving controlling + // terminal management is more than a little opaque and smacks of voodoo programming. errno = EINTR; while (result == -1 && errno == EINTR) { result = tcsetattr(STDIN_FILENO, TCSADRAIN, &j->tmodes); @@ -806,21 +818,32 @@ static bool terminal_give_to_job(job_t *j, int cont) { debug(1, _(L"terminal_give_to_job(): Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr()); wperror(L"tcsetattr"); + signal_unblock(true); return false; } } + signal_unblock(true); return true; } /// Returns control of the terminal to the shell, and saves the terminal attribute state to the job, /// so that we can restore the terminal ownership to the job at a later time. -static int terminal_return_from_job(job_t *j) { +static bool terminal_return_from_job(job_t *j) { + int s = 0; + errno = 0; + if (j->pgid == 0) { + debug(2, "terminal_return_from_job() returning early due to no process group"); + return true; + } + + signal_block(true); if (tcsetpgrp(STDIN_FILENO, getpgrp()) == -1) { if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not return shell to foreground")); wperror(L"tcsetpgrp"); - return 0; + signal_unblock(true); + return false; } // Save jobs terminal modes. @@ -828,7 +851,8 @@ static int terminal_return_from_job(job_t *j) { if (errno == EIO) redirect_tty_output(); debug(1, _(L"Could not return shell to foreground")); wperror(L"tcgetattr"); - return 0; + signal_unblock(true); + return false; } // Disabling this per @@ -841,11 +865,12 @@ static int terminal_return_from_job(job_t *j) { if (errno == EIO) redirect_tty_output(); debug(1, _(L"Could not return shell to foreground")); wperror(L"tcsetattr"); - return 0; + return false; } #endif - return 1; + signal_unblock(true); + return true; } void job_continue(job_t *j, bool cont) { @@ -854,7 +879,6 @@ void job_continue(job_t *j, bool cont) { j->set_flag(JOB_NOTIFIED, false); CHECK_BLOCK(); - debug(4, L"Continue job %d, gid %d (%ls), %ls, %ls", j->job_id, j->pgid, j->command_wcstr(), job_is_completed(j) ? L"COMPLETED" : L"UNCOMPLETED", is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE"); @@ -864,14 +888,7 @@ void job_continue(job_t *j, bool cont) { // Put the job into the foreground. Hack: ensure that stdin is marked as blocking first // (issue #176). make_fd_blocking(STDIN_FILENO); - - signal_block(); - - bool ok = terminal_give_to_job(j, cont); - - signal_unblock(); - - if (!ok) return; + if (!terminal_give_to_job(j, cont)) return; } // Send the job a continue signal, if necessary. @@ -955,15 +972,7 @@ void job_continue(job_t *j, bool cont) { // Put the shell back in the foreground. if (j->get_flag(JOB_TERMINAL) && j->get_flag(JOB_FOREGROUND)) { - int ok; - - signal_block(); - - ok = terminal_return_from_job(j); - - signal_unblock(); - - if (!ok) return; + terminal_return_from_job(j); } } } diff --git a/src/signal.cpp b/src/signal.cpp index 8cf5d01a3..93709a287 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -17,7 +17,7 @@ #include "wutil.h" // IWYU pragma: keep // This is a temporary var while we explore whether signal_block() and friends is needed. -bool ignore_signal_block = false; +bool ignore_signal_block = true; /// Struct describing an entry for the lookup table used to convert between signal names and signal /// ids, etc. @@ -372,8 +372,8 @@ void get_signals_with_handlers(sigset_t *set) { } } -void signal_block() { - if (ignore_signal_block) return; +void signal_block(bool force) { + if (!force && ignore_signal_block) return; ASSERT_IS_MAIN_THREAD(); sigset_t chldset; @@ -387,8 +387,8 @@ void signal_block() { // debug( 0, L"signal block level increased to %d", block_count ); } -void signal_unblock() { - if (ignore_signal_block) return; +void signal_unblock(bool force) { + if (!force && ignore_signal_block) return; ASSERT_IS_MAIN_THREAD(); sigset_t chldset; diff --git a/src/signal.h b/src/signal.h index 3c4da3990..436945105 100644 --- a/src/signal.h +++ b/src/signal.h @@ -27,10 +27,10 @@ void signal_set_handlers(); void signal_handle(int sig, int do_handle); /// Block all signals. -void signal_block(); +void signal_block(bool force = false); /// Unblock all signals. -void signal_unblock(); +void signal_unblock(bool force = false); /// Returns true if signals are being blocked. bool signal_is_blocked();