Subject: Re: timeout: Child gets SIGTTOU when run from a shell script

From: "Richard W.M. Jones" <[email-masked]>
Date: Fri, 1 Nov 2013 12:17:37 +0000

On Fri, Nov 01, 2013 at 11:53:44AM +0000, Pádraig Brady wrote:
> Probably something to do with job control
> If you `set -m` first in the script,
> then the test binary doesn't hang.

Unfortunately set -m isn't quite right for the script
we are interested in fixing:

https://github.com/libguestfs/libguestfs/blob/master/run.in#L217

> I did a version of timeout once that put the child in it's own group,
> and noted in that version that I needed to leave SIGTTOU as IGN
> as tcsetpgrp(0, getpid()) caused SIGTTOU to be sent and I wasn't sure why.

That sounds like a very similar situation to this one.  The
tty_check_change function is called all over the place, so just about
any ioctl or flush or tc* function could cause SIGTTOU to be sent.

> Maybe we should be resetting SIGTT{OU,IN} to what it was
> previously set to, rather than SIG_DFL?

The attached patch does *not* work .. don't know if you can see
any obvious mistakes.

Yesterday I looked through the strace -f output between the two cases
and came to the conclusion that it's not about signal handlers at all.
(What it's about is a mystery ...)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
--- src/timeout.c.old	2013-10-31 19:44:01.719755435 +0000
+++ src/timeout.c	2013-11-01 12:01:06.074839679 +0000
@@ -370,6 +370,7 @@
   double timeout;
   char signame[SIG2STR_MAX];
   int c;
+  struct sigaction sa, sa_ttin, sa_ttou;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -429,9 +430,16 @@
   /* Setup handlers before fork() so that we
      handle any signals caused by child, without races.  */
   install_signal_handlers (term_signal);
-  signal (SIGTTIN, SIG_IGN);   /* Don't stop if background child needs tty.  */
-  signal (SIGTTOU, SIG_IGN);   /* Don't stop if background child needs tty.  */
-  signal (SIGCHLD, SIG_DFL);   /* Don't inherit CHLD handling from parent.   */
+
+  /* Don't stop if background child needs tty.  */
+  sigemptyset (&sa.sa_mask);
+  sa.sa_handler = SIG_IGN;
+  sa.sa_flags = SA_RESTART;
+  sigaction (SIGTTIN, &sa, &sa_ttin);
+  sigaction (SIGTTOU, &sa, &sa_ttou);
+
+  /* Don't inherit CHLD handling from parent.   */
+  signal (SIGCHLD, SIG_DFL);
 
   monitored_pid = fork ();
   if (monitored_pid == -1)
@@ -443,9 +451,9 @@
     {                           /* child */
       int exit_status;
 
-      /* exec doesn't reset SIG_IGN -> SIG_DFL.  */
-      signal (SIGTTIN, SIG_DFL);
-      signal (SIGTTOU, SIG_DFL);
+      /* Restore old SIGTTIN/SIGTTOU handlers (RHBZ#1025269). */
+      sigaction (SIGTTIN, &sa_ttin, NULL);
+      sigaction (SIGTTOU, &sa_ttou, NULL);
 
       execvp (argv[0], argv);   /* FIXME: should we use "sh -c" ... here?  */