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.

3 files changed, 57 insertions, 22 deletions

diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index bac24dd1..00e6829e 100644
--- 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 f64b95dc..401cb78d 100644
--- 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 6f811ec1..a17b72d9 100644
--- 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