[OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Jackson]

This is all mad.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 standalone-generate-dump-flight-runvars |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/standalone-generate-dump-flight-runvars b/standalone-generate-dump-flight-runvars
index a1907b0..efedd5c 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -58,8 +58,32 @@ perbranch () {
     flight=check_${branch//[-._]/_}
 }
 
+# Good grief, handling background proceesses from shell is a pain.
+#
+# For stupid historical reasons, background processes start with
+# SIGINT (and QUIT) ignored (SuSv3 2.11).  bash does not offer a
+# way to ask it not to do this.
+#
+# There is no way to reset this in bash (bash 4.2.37 manpage section
+# on `trap' builtin), so we use perl.  However, there is still a race:
+# if the signal arrives just after the fork, after the shell has (in
+# the child) set it to to IGN, but before Perl has put it back, the
+# child might still escape.  So in the child we check our parent.
+#
+# I _think_ that that any signal which arrives before the assignment
+# to $SIG{} will definitely have caused our parent to vanish and us to
+# be reparented to pid 1 by the time we do the getppid check.  But TBH
+# I can't find any clear support for this requirement.  So the result
+# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
+# after starting.
+
 for branch in $@; do
     perbranch
+    perl -e '
+        $SIG{$_}=DFL foreach qw(INT QUIT HUP);
+	kill 1, $$ unless getppid=='$$';
+	exec @ARGV or die $!;
+    ' \
     ./standalone make-flight -f $flight $branch >$log 2>&1 &
     procs+=" $branch=$!"
 done
-- 
1.7.10.4

Re: [OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Jackson]

Ian Jackson writes ("[OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly"):
> This is all mad.
...
> +# I _think_ that that any signal which arrives before the assignment
> +# to $SIG{} will definitely have caused our parent to vanish and us to
> +# be reparented to pid 1 by the time we do the getppid check.  But TBH
> +# I can't find any clear support for this requirement.  So the result
> +# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
> +# after starting.

The test program below demonstrates that this assuption is not true.
However, a better approach is likely to be absurdly complex, involving
the parent shell having an INT trap which conducts an explicit
rendezvous with ?each child.

Ian.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <assert.h>
#include <sys/poll.h>
#include <sys/types.h>
#include <sys/wait.h>

static pid_t willdie, child, parent;

static void runtest(void) {
    willdie = getpid();  assert(willdie>=0);
    child = fork();  assert(child>=0);
    if (!child) {
        kill(willdie, SIGUSR1);
        parent = getppid();
        //        sleep(1);
        if (parent==1) _exit(0);
        fprintf(stderr,"willdie=%ld parent=%ld\n",
                (long)willdie,(long)parent);
        _exit(1);
    }
    for (;;)
        ;
}

int main(int argc, char **argv) {
    int st;

    for (;;) {
        willdie = fork();  assert(willdie>=0);
        if (!willdie) runtest();
        pid_t got = waitpid(willdie, &st, 0);
        assert(got==willdie);
        assert(WIFSIGNALED(st) && WTERMSIG(st)==SIGUSR1);
    }
}

Re: [OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Campbell]

On Fri, 2015-10-09 at 18:13 +0100, Ian Jackson wrote:
> This is all mad.

No kidding!

It doesn't appear to be possible to call setsid() from a shell script
(other than by re-execing oneself), which is a shame.

I wonder at what point we should just rewrite the whole thing in Perl?
Anyway right now:

> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  standalone-generate-dump-flight-runvars |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/standalone-generate-dump-flight-runvars b/standalone
> -generate-dump-flight-runvars
> index a1907b0..efedd5c 100755
> --- a/standalone-generate-dump-flight-runvars
> +++ b/standalone-generate-dump-flight-runvars
> @@ -58,8 +58,32 @@ perbranch () {
>      flight=check_${branch//[-._]/_}
>  }
>  
> +# Good grief, handling background proceesses from shell is a pain.

"processes"

> +#
> +# For stupid historical reasons, background processes start with
> +# SIGINT (and QUIT) ignored (SuSv3 2.11).  bash does not offer a
> +# way to ask it not to do this.
> +#
> +# There is no way to reset this in bash (bash 4.2.37 manpage section
> +# on `trap' builtin), so we use perl.  However, there is still a race:
> +# if the signal arrives just after the fork, after the shell has (in
> +# the child) set it to to IGN, but before Perl has put it back, the
> +# child might still escape.  So in the child we check our parent.
> +#
> +# I _think_ that that any signal which arrives before the assignment
> +# to $SIG{} will definitely have caused our parent to vanish and us to
> +# be reparented to pid 1 by the time we do the getppid check.  But TBH
> +# I can't find any clear support for this requirement.  So the result
> +# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
> +# after starting.

Based on your followup I guess you might want to clarify this?

> +
>  for branch in $@; do
>      perbranch
> +    perl -e '
> +        $SIG{$_}=DFL foreach qw(INT QUIT HUP);
> +	kill 1, $$ unless getppid=='$$';
> +	exec @ARGV or die $!;
> +    ' \
>      ./standalone make-flight -f $flight $branch >$log 2>&1 &
>      procs+=" $branch=$!"
>  done

[OSSTEST PATCH v2] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Jackson]

This is all mad.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Updated comment with more accurate discussion, references to
    bash "trap - INT" bug, and proposed new shopt.
    No code change.
---
 standalone-generate-dump-flight-runvars |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/standalone-generate-dump-flight-runvars b/standalone-generate-dump-flight-runvars
index a1907b0..e91026a 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -58,8 +58,35 @@ perbranch () {
     flight=check_${branch//[-._]/_}
 }
 
+# Good grief, handling background proceesses from shell is a pain.
+#
+# For stupid historical reasons, background processes start with
+# SIGINT (and QUIT) ignored (SuSv3 2.11).  bash does not currently
+# offer a way to ask it not to do this; nor is there a reliable way to
+# put the SIGINT handling back to normal.
+#
+# "trap - INT" can be used to put the handling back in recent versions
+# of bash (eg Debian jessie), but earlier versions are buggy, so
+# we use perl.
+#
+# However, there is still a race: if the signal arrives just after the
+# fork, after the shell has (in the child) set it to to IGN, but
+# before Perl has put it back, the child might still escape.
+# There is no reasonable way to deal with this race.  So the result
+# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
+# after starting.
+#
+# Hopefully in the future we can fix this with something like
+# "shopt -s no_async_sig_ignore".  See
+# https://lists.gnu.org/archive/html/bug-bash/2015-10/msg00077.html
+
 for branch in $@; do
     perbranch
+    perl -e '
+        $SIG{$_}=DFL foreach qw(INT QUIT HUP);
+	kill 1, $$ unless getppid=='$$';
+	exec @ARGV or die $!;
+    ' \
     ./standalone make-flight -f $flight $branch >$log 2>&1 &
     procs+=" $branch=$!"
 done
-- 
1.7.10.4