Debian Bug report logs - #607844
upstart: Use separate controlling terminal for tests

version graph

Package: upstart; Maintainer for upstart is Steve Langasek <vorlon@debian.org>; Source for upstart is src:upstart.

Reported by: Kees Cook <kees@debian.org>

Date: Wed, 22 Dec 2010 22:21:01 UTC

Severity: normal

Tags: fixed-upstream

Fixed in version 1.6-1

Done: Steve Langasek <vorlon@debian.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Wed, 22 Dec 2010 22:21:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kees Cook <kees@debian.org>:
New Bug report received and forwarded. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Wed, 22 Dec 2010 22:21:04 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Kees Cook <kees@debian.org>
To: Debian Bugs <submit@bugs.debian.org>
Subject: weird interaction between setsid and SIGTSTP
Date: Wed, 22 Dec 2010 14:17:45 -0800
[Message part 1 (text/plain, inline)]
Package: sbuild
Version: 0.60.5-1
Severity: normal
User: ubuntu-devel@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
[wstopped.c (text/x-csrc, attachment)]
[sbuild-poc (text/plain, attachment)]

Information forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Wed, 29 Dec 2010 15:51:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Roger Leigh <rleigh@codelibre.net>:
Extra info received and forwarded to list. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Wed, 29 Dec 2010 15:51:06 GMT) Full text and rfc822 format available.

Message #10 received at 607844@bugs.debian.org (full text, mbox):

From: Roger Leigh <rleigh@codelibre.net>
To: Kees Cook <kees@debian.org>, 607844@bugs.debian.org
Subject: Re: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
Date: Wed, 29 Dec 2010 15:46:11 +0000
[Message part 1 (text/plain, inline)]
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 (application/pgp-signature, inline)]

Information forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Mon, 30 May 2011 18:45:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Roger Leigh <rleigh@codelibre.net>:
Extra info received and forwarded to list. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Mon, 30 May 2011 18:45:03 GMT) Full text and rfc822 format available.

Message #15 received at 607844@bugs.debian.org (full text, mbox):

From: Roger Leigh <rleigh@codelibre.net>
To: Kees Cook <kees@debian.org>, 607844@bugs.debian.org
Subject: Re: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
Date: Mon, 30 May 2011 19:41:16 +0100
[Message part 1 (text/plain, inline)]
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 (application/pgp-signature, inline)]

Information forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Tue, 31 May 2011 20:51:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kees Cook <kees@debian.org>:
Extra info received and forwarded to list. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Tue, 31 May 2011 20:51:06 GMT) Full text and rfc822 format available.

Message #20 received at 607844@bugs.debian.org (full text, mbox):

From: Kees Cook <kees@debian.org>
To: Roger Leigh <rleigh@codelibre.net>
Cc: 607844@bugs.debian.org
Subject: Re: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
Date: Tue, 31 May 2011 13:46:30 -0700
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




Information forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Mon, 22 Oct 2012 14:09:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to james.hunt@ubuntu.com:
Extra info received and forwarded to list. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Mon, 22 Oct 2012 14:09:03 GMT) Full text and rfc822 format available.

Message #25 received at 607844@bugs.debian.org (full text, mbox):

From: James Hunt <james.hunt@ubuntu.com>
To: 607844@bugs.debian.org
Subject: weird interaction between setsid and SIGTSTP
Date: Mon, 22 Oct 2012 15:04:24 +0100
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



Information forwarded to debian-bugs-dist@lists.debian.org, Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>:
Bug#607844; Package sbuild. (Mon, 22 Oct 2012 14:36:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Roger Leigh <rleigh@codelibre.net>:
Extra info received and forwarded to list. Copy sent to Debian buildd-tools Developers <buildd-tools-devel@lists.alioth.debian.org>. (Mon, 22 Oct 2012 14:36:03 GMT) Full text and rfc822 format available.

Message #30 received at 607844@bugs.debian.org (full text, mbox):

From: Roger Leigh <rleigh@codelibre.net>
To: james.hunt@ubuntu.com, 607844@bugs.debian.org
Subject: Re: [buildd-tools-devel] Bug#607844: weird interaction between setsid and SIGTSTP
Date: Mon, 22 Oct 2012 15:33:14 +0100
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/Linux    http://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



Bug reassigned from package 'sbuild' to 'upstart'. Request was from Roger Leigh <rleigh@codelibre.net> to control@bugs.debian.org. (Mon, 22 Oct 2012 14:36:04 GMT) Full text and rfc822 format available.

No longer marked as found in versions sbuild/0.60.5-1. Request was from Roger Leigh <rleigh@codelibre.net> to control@bugs.debian.org. (Mon, 22 Oct 2012 14:36:05 GMT) Full text and rfc822 format available.

Changed Bug title to 'upstart: Use separate controlling terminal for tests' from 'weird interaction between setsid and SIGTSTP' Request was from Roger Leigh <rleigh@codelibre.net> to control@bugs.debian.org. (Mon, 22 Oct 2012 14:36:05 GMT) Full text and rfc822 format available.

Added tag(s) fixed-upstream. Request was from Roger Leigh <rleigh@codelibre.net> to control@bugs.debian.org. (Mon, 22 Oct 2012 14:36:05 GMT) Full text and rfc822 format available.

Reply sent to Steve Langasek <vorlon@debian.org>:
You have taken responsibility. (Sun, 02 Dec 2012 19:30:14 GMT) Full text and rfc822 format available.

Notification sent to Kees Cook <kees@debian.org>:
Bug acknowledged by developer. (Sun, 02 Dec 2012 19:30:15 GMT) Full text and rfc822 format available.

Message #43 received at 607844-done@bugs.debian.org (full text, mbox):

From: Steve Langasek <vorlon@debian.org>
To: 607844-done@bugs.debian.org
Subject: Re: upstart: Use separate controlling terminal for tests
Date: Sun, 2 Dec 2012 11:27:55 -0800
[Message part 1 (text/plain, inline)]
Version: 1.6-1

This bug is fixed upstream in upstart 1.6.

-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slangasek@ubuntu.com                                     vorlon@debian.org
[signature.asc (application/pgp-signature, inline)]

Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Tue, 26 Mar 2013 07:28:21 GMT) Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Thu Apr 17 11:28:00 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.