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? */
