Bug#607844: weird interaction between setsid and SIGTSTP
I've just committed a fix for this bug. See: https://bugs.launchpad.net/upstart/+bug/888910 Kind regards, James. -- James Hunt http://upstart.ubuntu.com/cookbook http://upstart.ubuntu.com/cookbook/upstart_cookbook.pdf -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#607844: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
reassign 607844 upstart retitle 607844 upstart: Use separate controlling terminal for tests tags 607844 + fixed-upstream thanks On Mon, Oct 22, 2012 at 03:04:24PM +0100, James Hunt wrote: I've just committed a fix for this bug. See: https://bugs.launchpad.net/upstart/+bug/888910 Thanks. Since this isn't an sbuild issue, I'll reassign to upstart. Thanks, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linuxhttp://people.debian.org/~rleigh/ `. `' schroot and sbuild http://alioth.debian.org/projects/buildd-tools `-GPG Public Key F33D 281D 470A B443 6756 147C 07B3 C8BC 4083 E800 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#607844: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
Hi Roger, On Mon, May 30, 2011 at 07:41:16PM +0100, Roger Leigh wrote: I was wondering whether or not this is still an issue for upstart? As far as I understand, the build is failing because we run dpkg-buildpackage without a controlling TTY. This has been the historical behaviour of sbuild (though for a short while the behaviour was removed). It's still a problem yes, but arguably, it is a problem with Upstart. It should only test controlling terminal logic if one is available. From the sbuild POV, the question is whether or not this is appropriate, and if we should continue to do this. However, while sbuild explicitly calls setsid, one consideration is that when run by buildd, buildd is itself already detached from its CTTY when it dæmonises, as a result any change in sbuild would only have an effect when run interactively. Right. If we were to alter sbuild's behaviour, we would have to create a PTY, run dpkg-buildpackage on the slave side and have sbuild record the build log from the master side. On the other hand, if upstart requires a CTTY for its tests, it would make sense for the test code to create a dedicated PTY for its tests to ensure that the tests will always run connected to a terminal. A simple openpty(3) would be sufficient if you dup() stdin/out/err for the child after fork so that the test runs on the PTY slave. I would tend to agree in this case. -Kees -- Kees Cook@debian.org -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#607844: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
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? Hi Kees, I was wondering whether or not this is still an issue for upstart? As far as I understand, the build is failing because we run dpkg-buildpackage without a controlling TTY. This has been the historical behaviour of sbuild (though for a short while the behaviour was removed). From the sbuild POV, the question is whether or not this is appropriate, and if we should continue to do this. However, while sbuild explicitly calls setsid, one consideration is that when run by buildd, buildd is itself already detached from its CTTY when it dæmonises, as a result any change in sbuild would only have an effect when run interactively. If we were to alter sbuild's behaviour, we would have to create a PTY, run dpkg-buildpackage on the slave side and have sbuild record the build log from the master side. On the other hand, if upstart requires a CTTY for its tests, it would make sense for the test code to create a dedicated PTY for its tests to ensure that the tests will always run connected to a terminal. A simple openpty(3) would be sufficient if you dup() stdin/out/err for the child after fork so that the test runs on the PTY slave. 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. signature.asc Description: Digital signature
Bug#607844: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
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. signature.asc Description: Digital signature
Bug#607844: weird interaction between setsid and SIGTSTP
Package: sbuild
Version: 0.60.5-1
Severity: normal
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu natty
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?
Thanks!
-Kees
--
Kees Cook@debian.org
#include sys/types.h
#include stdlib.h
#include sys/wait.h
#include stdio.h
#include signal.h
#include stdlib.h
#include unistd.h
void
test (void)
{
int fds[2];
if (pipe (fds) 0) {
perror (pipe);
exit (1);
}
pid_t pid = fork ();
if (pid 0) {
perror (fork);
exit (1);
} else if (pid == 0) {
close (fds[0]);
if (write (fds[1], \n, 1) 1) {
perror (write);
exit (1);
}
close (fds[1]);
raise (SIGTSTP);
exit (0);
} else if (pid 0) {
char buf[1];
close (fds[1]);
if (read (fds[0], buf, 1) 1) {
perror (read);
exit (1);
}
close (fds[0]);
siginfo_t info;
if (waitid (P_PID, pid, info, WSTOPPED | WNOWAIT) != 0) {
perror (waitid);
exit (1);
}
waitid (P_PID, pid, info, WSTOPPED);
if (kill (pid, SIGCONT) 0) {
perror (kill);
exit (1);
}
int status;
waitpid (pid, status, 0);
}
}
int
main (int argc, char *argv[])
{
test ();
puts(ok);
return 0;
}
#!/usr/bin/perl
# Author: Kees Cook k...@ubuntu.com
#
# This shows a strange failure with raise(SIGTSTP) when SETSID=1 during
# an sbuild run.
# Build up a package similar to Sbuild::Build
package Test::Build;
use strict;
use warnings;
use Sbuild::Base;
use Sbuild::ChrootRoot;
use Sbuild::Conf;
use Sbuild::Utility;
BEGIN {
use Exporter ();
our (@ISA, @EXPORT);
@ISA = qw(Exporter Sbuild::Base);
@EXPORT = qw();
}
sub new {
my $class = shift;
my $conf = Sbuild::Conf-new();
my $self = $class-SUPER::new($conf);
bless($self, $class);
return $self;
}
sub build {
my $self = shift;
my $cmd = shift;
my $setsid = shift;
$setsid += 0;
my $session = Sbuild::ChrootRoot-new($self-get('Config'));
$session-begin_session();
$self-set('Session', $session);
my $buildenv = {};
$buildenv-{'PATH'} = '/bin:/usr/bin';
my $command = {
COMMAND = [$cmd],
ENV = $buildenv,
SETSID = $setsid,
CHROOT = 1,
PRIORITY = 0,
DIR = '/tmp'
};
my $pipe = $self-get('Session')-pipe_command($command);
while($pipe) {
$self-log($_);
}
close($pipe);
}
package main;
my $test = Test::Build-new();
$test-build($ARGV[0], $ARGV[1]);
