From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 9E2601F8C6 for ; Sun, 7 Feb 2021 08:52:01 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 02/19] spawn: pi_fork_exec: support "pgid" Date: Sun, 7 Feb 2021 08:51:44 +0000 Message-Id: <20210207085201.13871-3-e@80x24.org> In-Reply-To: <20210207085201.13871-1-e@80x24.org> References: <20210207085201.13871-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- lib/PublicInbox/Spawn.pm | 37 +++++++++++++++++++++++++------------ lib/PublicInbox/SpawnPP.pm | 24 ++++++++++++++---------- t/spawn.t | 18 ++++++++++++++++++ 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