user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 02/19] spawn: pi_fork_exec: support "pgid"
Date: Sun,  7 Feb 2021 08:51:44 +0000	[thread overview]
Message-ID: <20210207085201.13871-3-e@80x24.org> (raw)
In-Reply-To: <20210207085201.13871-1-e@80x24.org>

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

  parent reply	other threads:[~2021-02-07  8:52 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07  8:51 [PATCH 00/19] lei import Maildir, remote mboxrd fixes Eric Wong
2021-02-07  8:51 ` [PATCH 01/19] spawn: pi_fork_exec: restore parent sigmask in child Eric Wong
2021-02-07  8:51 ` Eric Wong [this message]
2021-02-07 23:10   ` dprintf(3) portability? [was [02/19] spawn: pi_fork_exec: support "pgid"] Eric Wong
2021-02-07  8:51 ` [PATCH 03/19] lei add-external: handle interrupts with --mirror Eric Wong
2021-02-07  8:51 ` [PATCH 04/19] spawn_pp: die more consistently in child Eric Wong
2021-02-07  8:51 ` [PATCH 05/19] ipc: do not die inside wq_worker child process Eric Wong
2021-02-07  8:51 ` [PATCH 06/19] ipc: trim down the Storable checks Eric Wong
2021-02-07  8:51 ` [PATCH 07/19] Makefile.PL: depend on IO::Uncompress::Gunzip Eric Wong
2021-02-07  8:51 ` [PATCH 08/19] xapcmd: avoid potential die surprise in children Eric Wong
2021-02-07  8:51 ` [PATCH 09/19] tests: guard setup_public_inboxes for SQLite and Xapian Eric Wong
2021-02-07  8:51 ` [PATCH 10/19] Revert "ipc: add support for asynchronous callbacks" Eric Wong
2021-02-07  8:51 ` [PATCH 11/19] ipc: wq_do => wq_io_do Eric Wong
2021-02-07  8:51 ` [PATCH 12/19] lei: more consistent IPC exit and error handling Eric Wong
2021-02-07  8:51 ` [PATCH 13/19] lei: remove --mua-cmd alias for --mua Eric Wong
2021-02-07  8:51 ` [PATCH 14/19] lei: replace --thread with --threads Eric Wong
2021-02-07  8:51 ` [PATCH 15/19] lei q: improve remote mboxrd UX Eric Wong
2021-02-07  9:32   ` [PATCH 20/19] lei_xsearch: allow quieting regular mset progress, too Eric Wong
2021-02-07  8:51 ` [PATCH 16/19] lei q: SIGWINCH process group with the terminal Eric Wong
2021-02-07  8:51 ` [PATCH 17/19] lei import: support Maildirs Eric Wong
2021-02-07  8:52 ` [PATCH 18/19] imap: avoid unnecessary on-stack delete Eric Wong
2021-02-07  8:52 ` [PATCH 19/19] httpd/async: " Eric Wong
2021-02-07 10:40 ` [PATCH 21/19] lei q: fix arbitrary --mua command handling Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210207085201.13871-3-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    --subject='Re: [PATCH 02/19] spawn: pi_fork_exec: support "pgid"' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).