[buildd-tools-devel] Bug#607844: Bug#607844: weird interaction between setsid and SIGTSTP

[Roger Leigh]

On Wed, Dec 22, 2010 at 02:17:45PM -0800, Kees Cook wrote:
> While trying to build "upstart" in Ubuntu using the latest sbuild, one of
> the unit tests failed. This was reduced to a test of "raise(SIGTSTP)",
> which wasn't actually stopping the process. In tracking this down, I
> isolated it as far as setting SETSID to 1 during the Sbuild::Build phase.
> 
> Attached is "wstopped.c" and "sbuild-poc" which tries to isolate the
> problem. With SETSID=0, I get the expected behavior; the process waits for
> SIGCONT:
> 
> # perl /tmp/sbuild-poc /tmp/wstopped 0
> ok
> 
> With SETSID=1 (last cmdline option), the process does not wait for SIGCONT:
> 
> # perl /tmp/sbuild-poc /tmp/wstopped 1
> waitid: No child processes
> 
> The bulk of wstopped.c is the creation of IPC pipes to make sure the child
> has started and the parent can wait on it.
> 
> Very weird. Do you have any idea what is going on here?

Yes, but my understanding may be incomplete…

After the setsid(2) call, the dpkg-buildpackage command is run in its
own separate session, with dpkg-buildpackage being the process group
leader.  A side-effect of setsid is that the process group has no
controlling TTY.  A consequence of having no CTTY is that the terminal
process control signals such as SIGTSTP (and also SIGTTIN/TTOU) won't
work--there's no tty to signal the group leader [though I admit I don't
know what happens when there's no TTY; apparently nothing from what
you've described].

This change was introduced in commit 8f254221.  However, setsid was
used routinely until removed in c4a3bd69.  There was a reason for
having the setsid call, though it's something I've never been totally
happy about.  It's so we can terminate the entire dpkg-buildpackage
process group without killing ourselves (since otherwise we're in the
same group and cleanup is messy--we can kill dpkg-buildpackage but not
any of its children, so the build can continue on unless we kill the
entire group as a shell would).  Killing the entire process group in
a single shot is much more robust.

From the upstart perspective, it's lacking a ctty.  However, having a
controlling tty is not something we have ever guaranteed; we don't
even guarantee having a terminal (since stdin/out are all redirected
to /dev/null or logfiles--no interactivity is allowed).

Suggestions:
- upstart should check if it has a controlling terminal.  ttyname(2)
  will return NULL if none is present.  IMHO, upstart should work
  in the absence of a controlling terminal; if it needs one for the
  unit tests, it could either skip those tests or (better) create a
  pty to run the tests on which would be much more robust all around
  since you have full control over what's running on it.

- sbuild /could/ run the build connected to a pseudo-TTY.  The build
  can run on the pty slave and we can control the master for logging
  etc.  However, allowing terminal signals means builds can raise
  signals and freeze a build indefinitely; this is also a reason why
  we don't have a ctty right now--it means the buildds need less
  manual cleanup if a build stops itself.  I've considered doing this
  several times in the past, but I'm yet to be convinced it adds any
  useful value.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20101229/aaa62614/attachment.pgp>