terminal_maybe_give_to_job to stop returning error on ENOTTY

Prior to this fix, if job control is enabled but stdin is not a tty, we
would return an error from terminal_maybe_give_to_job which would cause us
to avoid waiting for the job. Instead just return notneeded.

Fixes #6573.

src/proc.cpp

@@ -763,7 +763,7 @@ int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
         return notneeded;
     }
 
-    if (j->pgid == 0) {
+    if (j->pgid == INVALID_PID || j->pgid == 0) {
         FLOG(proc_termowner, L"terminal_give_to_job() returning early due to no process group");
         return notneeded;
     }
@@ -783,7 +783,22 @@ int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
     // http://curiousthing.org/sigttin-sigttou-deep-dive-linux In all cases, our goal here was just
     // to hand over control of the terminal to this process group, which is a no-op if it's already
     // been done.
-    if (j->pgid == INVALID_PID || tcgetpgrp(STDIN_FILENO) == j->pgid) {
+    int getpgrp_res = tcgetpgrp(STDIN_FILENO);
+    if (getpgrp_res < 0) {
+        if (errno == ENOTTY) {
+            // stdin is not a tty. This may come about if job control is enabled but we are not a
+            // tty - see #6573.
+            return notneeded;
+        } else if (errno == EBADF) {
+            // stdin has been closed. Workaround a glibc bug - see #3644.
+            redirect_tty_output();
+            return notneeded;
+        }
+        wperror(L"tcgetpgrp");
+        return error;
+    }
+    assert(getpgrp_res >= 0);
+    if (getpgrp_res == j->pgid) {
         FLOGF(proc_termowner, L"Process group %d already has control of terminal", j->pgid);
     } else {
         FLOGF(proc_termowner,
@@ -803,7 +818,6 @@ int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
             FLOGF(proc_termowner, L"tcsetpgrp failed: %d", errno);
 
             bool pgroup_terminated = false;
-            // No need to test for EINTR as we are blocking signals
             if (errno == EINVAL) {
                 // OS X returns EINVAL if the process group no longer lives. Probably other OSes,
                 // too. Unlike EPERM below, EINVAL can only happen if the process group has
@@ -827,7 +841,9 @@ int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
                 }
             } else {
                 if (errno == ENOTTY) {
-                    redirect_tty_output();
+                    // stdin is not a TTY. In general we expect this to be caught via the tcgetpgrp
+                    // call.
+                    return notneeded;
                 } else {
                     FLOGF(warning, _(L"Could not send job %d ('%ls') with pgid %d to foreground"),
                           j->job_id(), j->command_wcstr(), j->pgid);
@@ -854,15 +870,7 @@ int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
     if (continuing_from_stopped) {
         auto result = tcsetattr(STDIN_FILENO, TCSADRAIN, &j->tmodes);
         if (result == -1) {
-            // No need to test for EINTR and retry since we have blocked all signals
-            if (errno == ENOTTY) {
-                redirect_tty_output();
-            }
-
-            FLOGF(warning, _(L"Could not send job %d ('%ls') to foreground"), j->job_id(),
-                  j->preview().c_str());
             wperror(L"tcsetattr");
-            return error;
         }
     }
 

tests/checks/job-control-not-a-tty.fish

@@ -0,0 +1,6 @@
+#RUN: echo 'status job-control full; command echo A ; echo B;' | %fish
+
+# Regression test for #6573.
+
+#CHECK: A
+#CHECK: B