spawn: pi_fork_exec: support "pgid"

We'll be using this to allow the "git clone" process hierarchy
to be killed via Ctrl-C.  This also fixes a long-standing bug
in error reporting for the Inline::C version, because we're
actually testing for errors, now!

n.b. strlen(3) is officially async-signal-safe as of
POSIX.1-2016, but I can't think of a reason any previous
implementation prior to that wouldn't be.

diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index bac24dd1a7c883dd2f4ba94161ce25b884c2acd9..00e6829e9483918c92bccefef153c1477c084b2f 100644 (file)
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -61,9 +61,10 @@ my $all_libc = <<'ALL_LIBC'; # all *nix systems we support
 } while (0)
 
 /* needs to be safe inside a vfork'ed process */
-static void exit_err(int *cerrnum)
+static void exit_err(const char *fn, volatile int *cerrnum)
 {
        *cerrnum = errno;
+       write(2, fn, strlen(fn));
        _exit(1);
 }
 
@@ -73,7 +74,7 @@ static void exit_err(int *cerrnum)
  * Be sure to update PublicInbox::SpawnPP if this changes
  */
 int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
-                const char *cd)
+                const char *cd, int pgid)
 {
        AV *redir = (AV *)SvRV(redirref);
        AV *cmd = (AV *)SvRV(cmdref);
@@ -83,8 +84,10 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
        pid_t pid;
        char **argv, **envp;
        sigset_t set, old;
-       int ret, perrnum, cerrnum = 0;
+       int ret, perrnum;
+       volatile int cerrnum = 0; /* shared due to vfork */
        int chld_is_member;
+       I32 max_fd = av_len(redir);
 
        AV2C_COPY(argv, cmd);
        AV2C_COPY(envp, env);
@@ -99,23 +102,25 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
        pid = vfork();
        if (pid == 0) {
                int sig;
-               I32 i, child_fd, max = av_len(redir);
+               I32 i, child_fd, max_rlim;
 
-               for (child_fd = 0; child_fd <= max; child_fd++) {
+               for (child_fd = 0; child_fd <= max_fd; child_fd++) {
                        SV **parent = av_fetch(redir, child_fd, 0);
                        int parent_fd = SvIV(*parent);
                        if (parent_fd == child_fd)
                                continue;
                        if (dup2(parent_fd, child_fd) < 0)
-                               exit_err(&cerrnum);
+                               exit_err("dup2", &cerrnum);
                }
+               if (pgid >= 0 && setpgid(0, pgid) < 0)
+                       exit_err("setpgid", &cerrnum);
                for (sig = 1; sig < NSIG; sig++)
                        signal(sig, SIG_DFL); /* ignore errors on signals */
                if (*cd && chdir(cd) < 0)
-                       exit_err(&cerrnum);
+                       exit_err("chdir", &cerrnum);
 
-               max = av_len(rlim);
-               for (i = 0; i < max; i += 3) {
+               max_rlim = av_len(rlim);
+               for (i = 0; i < max_rlim; i += 3) {
                        struct rlimit rl;
                        SV **res = av_fetch(rlim, i, 0);
                        SV **soft = av_fetch(rlim, i + 1, 0);
@@ -124,12 +129,12 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
                        rl.rlim_cur = SvIV(*soft);
                        rl.rlim_max = SvIV(*hard);
                        if (setrlimit(SvIV(*res), &rl) < 0)
-                               exit_err(&cerrnum);
+                               exit_err("setrlimit", &cerrnum);
                }
 
                (void)sigprocmask(SIG_SETMASK, &old, NULL);
                execve(filename, argv, envp);
-               exit_err(&cerrnum);
+               exit_err("execve", &cerrnum);
        }
        perrnum = errno;
        if (chld_is_member > 0)
@@ -137,9 +142,16 @@ int pi_fork_exec(SV *redirref, SV *file, SV *cmdref, SV *envref, SV *rlimref,
        ret = sigprocmask(SIG_SETMASK, &old, NULL);
        assert(ret == 0 && "BUG calling sigprocmask to restore");
        if (cerrnum) {
+               int err_fd = STDERR_FILENO;
+               if (err_fd <= max_fd) {
+                       SV **parent = av_fetch(redir, err_fd, 0);
+                       err_fd = SvIV(*parent);
+               }
                if (pid > 0)
                        waitpid(pid, NULL, 0);
                pid = -1;
+               /* continue message started by exit_err in child */
+               dprintf(err_fd, ": %s\n", strerror(cerrnum));
                errno = cerrnum;
        } else if (perrnum) {
                errno = perrnum;
@@ -373,7 +385,8 @@ sub spawn ($;$$) {
                push @$rlim, $r, @$v;
        }
        my $cd = $opts->{'-C'} // ''; # undef => NULL mapping doesn't work?
-       my $pid = pi_fork_exec($redir, $f, $cmd, \@env, $rlim, $cd);
+       my $pgid = $opts->{pgid} // -1;
+       my $pid = pi_fork_exec($redir, $f, $cmd, \@env, $rlim, $cd, $pgid);
        die "fork_exec @$cmd failed: $!\n" unless $pid > 0;
        $pid;
 }

diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm
index f64b95dcdefc5feb12d17676c04764e9e61c6140..401cb78de083ebf294ddec7847294d849d1c1142 100644 (file)
--- a/lib/PublicInbox/SpawnPP.pm
+++ b/lib/PublicInbox/SpawnPP.pm
@@ -5,12 +5,12 @@
 # of vfork, so no speedups under Linux for spawning from large processes.
 package PublicInbox::SpawnPP;
 use strict;
-use warnings;
-use POSIX qw(dup2 :signal_h);
+use v5.10.1;
+use POSIX qw(dup2 _exit setpgid :signal_h);
 
 # Pure Perl implementation for folks that do not use Inline::C
-sub pi_fork_exec ($$$$$$) {
-       my ($redir, $f, $cmd, $env, $rlim, $cd) = @_;
+sub pi_fork_exec ($$$$$$$) {
+       my ($redir, $f, $cmd, $env, $rlim, $cd, $pgid) = @_;
        my $old = POSIX::SigSet->new();
        my $set = POSIX::SigSet->new();
        $set->fillset or die "fillset failed: $!";
@@ -22,21 +22,25 @@ sub pi_fork_exec ($$$$$$) {
                $pid = -1;
        }
        if ($pid == 0) {
-               while (@$rlim) {
-                       my ($r, $soft, $hard) = splice(@$rlim, 0, 3);
-                       BSD::Resource::setrlimit($r, $soft, $hard) or
-                         warn "failed to set $r=[$soft,$hard]\n";
-               }
                for my $child_fd (0..$#$redir) {
                        my $parent_fd = $redir->[$child_fd];
                        next if $parent_fd == $child_fd;
                        dup2($parent_fd, $child_fd) or
                                die "dup2($parent_fd, $child_fd): $!\n";
                }
+               if ($pgid >= 0 && !defined(setpgid(0, $pgid))) {
+                       warn "setpgid: $!";
+                       _exit(1);
+               }
+               $SIG{$_} = 'DEFAULT' for keys %SIG;
                if ($cd ne '') {
                        chdir $cd or die "chdir $cd: $!";
                }
-               $SIG{$_} = 'DEFAULT' for keys %SIG;
+               while (@$rlim) {
+                       my ($r, $soft, $hard) = splice(@$rlim, 0, 3);
+                       BSD::Resource::setrlimit($r, $soft, $hard) or
+                         warn "failed to set $r=[$soft,$hard]\n";
+               }
                $old->delset(POSIX::SIGCHLD) or die "delset SIGCHLD: $!";
                sigprocmask(SIG_SETMASK, $old) or die "SETMASK: ~SIGCHLD: $!";
                if ($ENV{MOD_PERL}) {

diff --git a/t/spawn.t b/t/spawn.t
index 6f811ec163df75fab559a2c119b723daad23f27f..a17b72d97ff5926d41477647b0ffc273d16d5dc2 100644 (file)
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -18,6 +18,24 @@ use PublicInbox::Sigfd;
        is($?, 0, 'true exited successfully');
 }
 
+SKIP: {
+       my $pid = spawn(['true'], undef, { pgid => 0 });
+       ok($pid, 'spawned process with new pgid');
+       is(waitpid($pid, 0), $pid, 'waitpid succeeds on spawned process');
+       is($?, 0, 'true exited successfully');
+       pipe(my ($r, $w)) or BAIL_OUT;
+       $pid = eval { spawn(['true'], undef, { pgid => 1, 2 => $w }) };
+       close $w;
+       my $err = do { local $/; <$r> };
+       # diag "$err ($@)";
+       if (defined $pid) {
+               waitpid($pid, 0) if defined $pid;
+               isnt($?, 0, 'child error (pure-Perl)');
+       } else {
+               ok($@, 'exception raised');
+       }
+}
+
 { # ensure waitpid(-1, 0) and SIGCHLD works in spawned process
        my $script = <<'EOF';
 $| = 1; # unbuffer stdout