Place fish in its own process group when launched with -i

Fixes #5909

src/builtin.cpp

@@ -449,7 +449,8 @@ proc_status_t builtin_run(parser_t &parser, int job_pgid, wchar_t **argv, io_str
     if (const builtin_data_t *data = builtin_lookup(argv[0])) {
         // If we are interactive, save the foreground pgroup and restore it after in case the
         // builtin needs to read from the terminal. See #4540.
-        bool grab_tty = is_interactive_session() && isatty(streams.stdin_fd);
+        bool grab_tty = session_interactivity() != session_interactivity_t::not_interactive &&
+                        isatty(streams.stdin_fd);
         pid_t pgroup_to_restore = grab_tty ? terminal_acquire_before_builtin(job_pgid) : -1;
         int ret = data->func(parser, streams, argv);
         if (pgroup_to_restore >= 0) {

src/builtin_commandline.cpp

@@ -160,7 +160,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
     }
 
     if (!current_buffer) {
-        if (is_interactive_session()) {
+        if (session_interactivity() != session_interactivity_t::not_interactive) {
             // Prompt change requested while we don't have a prompt, most probably while reading the
             // init files. Just ignore it.
             return STATUS_CMD_ERROR;

src/builtin_status.cpp

@@ -384,7 +384,7 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         }
         case STATUS_IS_INTERACTIVE: {
             CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd)
-            retval = !is_interactive_session();
+            retval = session_interactivity() == session_interactivity_t::not_interactive ? 1 : 0;
             break;
         }
         case STATUS_IS_COMMAND_SUB: {

src/env_dispatch.cpp

@@ -394,11 +394,12 @@ static bool initialize_curses_using_fallback(const char *term) {
     auto term_env = wcs2string(term_var->as_string());
     if (term_env == DEFAULT_TERM1 || term_env == DEFAULT_TERM2) return false;
 
-    if (is_interactive_session()) debug(1, _(L"Using fallback terminal type '%s'."), term);
+    if (session_interactivity() != session_interactivity_t::not_interactive)
+        debug(1, _(L"Using fallback terminal type '%s'."), term);
 
     int err_ret;
     if (setupterm(const_cast<char *>(term), STDOUT_FILENO, &err_ret) == OK) return true;
-    if (is_interactive_session()) {
+    if (session_interactivity() != session_interactivity_t::not_interactive) {
         debug(1, _(L"Could not set up terminal using the fallback terminal type '%s'."), term);
     }
     return false;
@@ -457,7 +458,7 @@ static void init_curses(const environment_t &vars) {
     int err_ret;
     if (setupterm(nullptr, STDOUT_FILENO, &err_ret) == ERR) {
         auto term = vars.get(L"TERM");
-        if (is_interactive_session()) {
+        if (session_interactivity() != session_interactivity_t::not_interactive) {
             debug(1, _(L"Could not set up terminal."));
             if (term.missing_or_empty()) {
                 debug(1, _(L"TERM environment variable not set."));

src/fish.cpp

@@ -396,7 +396,7 @@ static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) {
     // command or file to execute and stdin is a tty. Note that the -i or
     // --interactive options also force interactive mode.
     if (opts->batch_cmds.empty() && optind == argc && isatty(STDIN_FILENO)) {
-        set_interactive_session(true);
+        set_interactive_session(session_interactivity_t::implied);
     }
 
     return optind;
@@ -446,11 +446,11 @@ int main(int argc, char **argv) {
     // Apply our options.
     if (opts.is_login) mark_login();
     if (opts.no_exec) mark_no_exec();
-    if (opts.is_interactive_session) set_interactive_session(true);
+    if (opts.is_interactive_session) set_interactive_session(session_interactivity_t::explicit_);
 
     // Only save (and therefore restore) the fg process group if we are interactive. See issues
     // #197 and #1002.
-    if (is_interactive_session()) {
+    if (session_interactivity() != session_interactivity_t::not_interactive) {
         save_term_foreground_process_group();
     }
 

src/fish_key_reader.cpp

@@ -287,7 +287,8 @@ static void install_our_signal_handlers() {
 
 /// Setup our environment (e.g., tty modes), process key strokes, then reset the environment.
 static void setup_and_process_keys(bool continuous_mode) {
-    set_interactive_session(true);  // by definition this program is interactive
+    set_interactive_session(
+        session_interactivity_t::implied);  // by definition this program is interactive
     set_main_thread();
     setup_fork_guards();
     env_init();

src/fish_tests.cpp

@@ -1069,10 +1069,10 @@ static void test_cancellation() {
 
     // Ensure that if child processes SIGINT, we exit our loops
     // Test for #3780
-    // Ugly hack - temporarily set is_interactive_session
+    // Ugly hack - temporarily fake an interactive session
     // else we will SIGINT ourselves in response to our child death
-    bool iis = is_interactive_session();
+    session_interactivity_t iis = session_interactivity();
-    set_interactive_session(true);
+    set_interactive_session(session_interactivity_t::implied);
     const wchar_t *child_self_destructor = L"while true ; sh -c 'sleep .25; kill -s INT $$' ; end";
     parser_t::principal_parser().eval(child_self_destructor, io_chain_t());
     set_interactive_session(iis);

src/proc.cpp

@@ -54,9 +54,10 @@
 /// The signals that signify crashes to us.
 static const int crashsignals[] = {SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGSYS};
 
-static relaxed_atomic_bool_t s_is_interactive_session{false};
+static relaxed_atomic_t<session_interactivity_t> s_is_interactive_session{
-bool is_interactive_session() { return s_is_interactive_session; }
+    session_interactivity_t::not_interactive};
-void set_interactive_session(bool flag) { s_is_interactive_session = flag; }
+session_interactivity_t session_interactivity() { return s_is_interactive_session; }
+void set_interactive_session(session_interactivity_t flag) { s_is_interactive_session = flag; }
 
 static relaxed_atomic_bool_t s_is_login{false};
 bool get_login() { return s_is_login; }
@@ -245,7 +246,7 @@ static void handle_child_status(process_t *proc, proc_status_t status) {
     if (status.signal_exited()) {
         int sig = status.signal_code();
         if (sig == SIGINT || sig == SIGQUIT) {
-            if (is_interactive_session()) {
+            if (session_interactivity() != session_interactivity_t::not_interactive) {
                 // In an interactive session, tell the principal parser to skip all blocks we're
                 // executing so control-C returns control to the user.
                 parser_t::skip_all_blocks();

src/proc.h

@@ -475,8 +475,9 @@ class job_t {
 };
 
 /// Whether this shell is attached to the keyboard at all.
-bool is_interactive_session();
+enum class session_interactivity_t { not_interactive, implied, explicit_ };
-void set_interactive_session(bool flag);
+session_interactivity_t session_interactivity();
+void set_interactive_session(session_interactivity_t flag);
 
 /// Whether we are a login shell.
 bool get_login();

src/reader.cpp

@@ -1734,7 +1734,7 @@ static void reader_interactive_init(parser_t &parser) {
                 owner = tcgetpgrp(STDIN_FILENO);
             }
             if (owner == -1 && errno == ENOTTY) {
-                if (!is_interactive_session()) {
+                if (session_interactivity() == session_interactivity_t::not_interactive) {
                     // It's OK if we're not able to take control of the terminal. We handle
                     // the fallout from this in a few other places.
                     break;
@@ -1772,7 +1772,8 @@ static void reader_interactive_init(parser_t &parser) {
     // It shouldn't be necessary to place fish in its own process group and force control
     // of the terminal, but that works around fish being started with an invalid pgroup,
     // such as when launched via firejail (#5295)
-    if (shell_pgid == 0) {
+    // Also become the process group leader if flag -i/--interactive was given (#5909).
+    if (shell_pgid == 0 || session_interactivity() == session_interactivity_t::explicit_) {
         shell_pgid = getpid();
         if (setpgid(shell_pgid, shell_pgid) < 0) {
             FLOG(error, _(L"Failed to assign shell to its own process group"));
@@ -3239,7 +3240,7 @@ maybe_t<wcstring> reader_data_t::readline(int nchars_or_0) {
         // This check is required to work around certain issues with fish's approach to
         // terminal control when launching interactive processes while in non-interactive
         // mode. See #4178 for one such example.
-        if (err != ENOTTY || is_interactive_session()) {
+        if (err != ENOTTY || session_interactivity() != session_interactivity_t::not_interactive) {
             wperror(L"tcsetattr");
         }
     }
@@ -3342,7 +3343,8 @@ maybe_t<wcstring> reader_data_t::readline(int nchars_or_0) {
     if (!reader_exit_forced()) {
         // The order of the two conditions below is important. Try to restore the mode
         // in all cases, but only complain if interactive.
-        if (tcsetattr(0, TCSANOW, &old_modes) == -1 && is_interactive_session()) {
+        if (tcsetattr(0, TCSANOW, &old_modes) == -1 &&
+            session_interactivity() != session_interactivity_t::not_interactive) {
             if (errno == EIO) redirect_tty_output();
             wperror(L"tcsetattr");  // return to previous mode
         }