Claim the tty unconditionally in reader_data_t::readline

When fish runs with job control enabled, it transfers ownership of the
tty to a child process, and then reclaims the tty after the process
exits. If job control is disabled then fish does not transfer or reclaim
the tty.

It may happen that the child process creates a pgroup and then transfers
the tty to it. In that case fish will not attempt to reclaim the tty, as
fish did not transfer it. Then when fish reads from stdin it will
receive SIGTTIN instead of data.

Fix this by unconditionally claiming the tty in readline().

Fixes #9181

CHANGELOG.rst

@@ -69,6 +69,7 @@ Interactive improvements
 - A crash when completing a token that contained both a potential glob and a quoted variable expansion was fixed (:issue:`9137`).
 - If ``$fish_color_valid_path`` contains an actual color instead of just modifiers, those will be used for valid paths even if the underlying color isn't "normal" (:issue:`9159`).
 - ``prompt_pwd`` no longer accidentally overwrites a global or universal ``$fish_prompt_pwd_full_dirs`` when called with the ``-d`` or ``--full-length-dirs`` option (:issue:`9123`).
+- A bug which caused fish to freeze or exit after running a command which does not preserve the foreground process group was fixed (:issue:`9181`).
 
 New or improved bindings
 ^^^^^^^^^^^^^^^^^^^^^^^^

src/fish_test_helper.cpp

@@ -2,6 +2,7 @@
 // programs, allowing fish to test its behavior.
 
 #include <fcntl.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #include <algorithm>
@@ -11,6 +12,27 @@
 #include <cstring>
 #include <iterator>  // for std::begin/end
 
+static void abandon_tty() {
+    pid_t pid = fork();
+    if (pid < 0) {
+        perror("fork");
+        exit(EXIT_FAILURE);
+    }
+    // Both parent and child do the same thing.
+    pid_t child = pid ? pid : getpid();
+    if (setpgid(child, child)) {
+        perror("setpgid");
+        exit(EXIT_FAILURE);
+    }
+    // tcsetpgrp may fail in the parent if the child has already exited.
+    // This is the benign race.
+    (void)tcsetpgrp(STDIN_FILENO, child);
+    // Parent waits for child to exit.
+    if (pid > 0) {
+        waitpid(child, nullptr, 0);
+    }
+}
+
 static void become_foreground_then_print_stderr() {
     if (tcsetpgrp(STDOUT_FILENO, getpgrp()) < 0) {
         perror("tcsetgrp");
@@ -192,6 +214,7 @@ struct fth_command_t {
 };
 
 static fth_command_t s_commands[] = {
+    {"abandon_tty", abandon_tty, "Create a new pgroup and transfer tty ownership to it"},
     {"become_foreground_then_print_stderr", become_foreground_then_print_stderr,
      "Claim the terminal (tcsetpgrp) and then print to stderr"},
     {"nohup_wait", nohup_wait, "Ignore SIGHUP and just wait"},

src/reader.cpp

@@ -4278,6 +4278,11 @@ maybe_t<wcstring> reader_data_t::readline(int nchars_or_0) {
 
     history_search.reset();
 
+    // It may happen that a command we ran when job control was disabled nevertheless stole the tty
+    // from us. In that case when we read from our fd, it will trigger SIGTTIN. So just
+    // unconditionally reclaim the tty. See #9181.
+    (void)tcsetpgrp(conf.in, getpgrp());
+
     // Get the current terminal modes. These will be restored when the function returns.
     struct termios old_modes {};
     if (tcgetattr(conf.in, &old_modes) == -1 && errno == EIO) redirect_tty_output();

tests/pexpects/tty_ownership.py

@@ -15,3 +15,15 @@ expect_prompt()
 
 sendline("echo it worked")
 expect_prompt("it worked")
+
+# Regression test for #9181
+sendline("status job-control interactive")
+expect_prompt()
+sendline("$fish_test_helper abandon_tty")
+expect_prompt()
+sendline("echo cool")
+expect_prompt("cool")
+sendline("true ($fish_test_helper abandon_tty)")
+expect_prompt()
+sendline("echo even cooler")
+expect_prompt("even cooler")