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 21/26] lei: drop $SIG{__DIE__}, add oneshot fallbacks
Date: Fri, 18 Dec 2020 12:09:45 +0000	[thread overview]
Message-ID: <20201218120950.23272-22-e@80x24.org> (raw)
In-Reply-To: <20201218120950.23272-1-e@80x24.org>

We'll force stdout+stderr to be a pipe the spawning client
controls, thus there's no need to lose error reporting by
prematurely redirecting stdout+stderr to /dev/null.

We can now rely exclusively on OnDestroy to write to syslog() on
uncaught die failures.

Also support falling back to oneshot mode on socket and cwd
failures, since some commands may still be useful if the current
working directory goes missing :P
---
 lib/PublicInbox/LEI.pm | 67 ++++++++++++++++++++----------------------
 script/lei             | 39 +++++++++++++++---------
 t/lei.t                | 31 +++++++++++++++++--
 3 files changed, 86 insertions(+), 51 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 95b48095..fd412324 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -12,7 +12,7 @@ use parent qw(PublicInbox::DS);
 use Getopt::Long ();
 use Socket qw(AF_UNIX SOCK_STREAM pack_sockaddr_un);
 use Errno qw(EAGAIN ECONNREFUSED ENOENT);
-use POSIX qw(setsid);
+use POSIX ();
 use IO::Handle ();
 use Sys::Syslog qw(syslog openlog);
 use PublicInbox::Config;
@@ -584,60 +584,44 @@ sub noop {}
 
 # lei(1) calls this when it can't connect
 sub lazy_start {
-	my ($path, $err) = @_;
-	require IO::FDPass; # require this early so caller sees it
-	if ($err == ECONNREFUSED) {
+	my ($path, $errno) = @_;
+	if ($errno == ECONNREFUSED) {
 		unlink($path) or die "unlink($path): $!";
-	} elsif ($err != ENOENT) {
-		$! = $err; # allow interpolation to stringify in die
+	} elsif ($errno != ENOENT) {
+		$! = $errno; # allow interpolation to stringify in die
 		die "connect($path): $!";
 	}
 	umask(077) // die("umask(077): $!");
 	socket(my $l, AF_UNIX, SOCK_STREAM, 0) or die "socket: $!";
 	bind($l, pack_sockaddr_un($path)) or die "bind($path): $!";
-	listen($l, 1024) or die "listen $!";
+	listen($l, 1024) or die "listen: $!";
 	my @st = stat($path) or die "stat($path): $!";
 	my $dev_ino_expect = pack('dd', $st[0], $st[1]); # dev+ino
 	pipe(my ($eof_r, $eof_w)) or die "pipe: $!";
 	my $oldset = PublicInbox::Sigfd::block_signals();
-	my $pid = fork // die "fork: $!";
-	return if $pid;
+	require IO::FDPass;
 	require PublicInbox::Listener;
 	require PublicInbox::EOFpipe;
-	openlog($path, 'pid', 'user');
-	local $SIG{__DIE__} = sub {
-		syslog('crit', "@_");
-		die; # calls the default __DIE__ handler
-	};
-	local $SIG{__WARN__} = sub { syslog('warning', "@_") };
-	open(STDIN, '+<', '/dev/null') or die "redirect stdin failed: $!\n";
-	open STDOUT, '>&STDIN' or die "redirect stdout failed: $!\n";
-	open STDERR, '>&STDIN' or die "redirect stderr failed: $!\n";
-	setsid();
-	$pid = fork // die "fork: $!";
+	(-p STDOUT && -p STDERR) or die "E: stdout+stderr must be pipes\n";
+	open(STDIN, '+<', '/dev/null') or die "redirect stdin failed: $!";
+	POSIX::setsid() > 0 or die "setsid: $!";
+	my $pid = fork // die "fork: $!";
 	return if $pid;
-	$SIG{__DIE__} = 'DEFAULT';
-	my $on_destroy = PublicInbox::OnDestroy->new(sub {
-		my ($owner_pid) = @_;
-		syslog('crit', "$@") if $@ && $$ == $owner_pid;
-	}, $$);
 	$0 = "lei-daemon $path";
 	local %PATH2CFG;
-	$l->blocking(0);
-	$eof_w->blocking(0);
-	$eof_r->blocking(0);
-	my $listener = PublicInbox::Listener->new($l, \&accept_dispatch, $l);
+	$_->blocking(0) for ($l, $eof_r, $eof_w);
+	$l = PublicInbox::Listener->new($l, \&accept_dispatch, $l);
 	my $exit_code;
 	local $quit = sub {
 		$exit_code //= shift;
-		my $tmp = $listener or exit($exit_code);
+		my $listener = $l or exit($exit_code);
 		unlink($path) if defined($path);
-		syswrite($eof_w, '.');
-		$l = $listener = $path = undef;
-		$tmp->close if $tmp; # DS::close
+		# closing eof_w triggers \&noop wakeup
+		$eof_w = $l = $path = undef;
+		$listener->close; # DS::close
 		PublicInbox::DS->SetLoopTimeout(1000);
 	};
-	PublicInbox::EOFpipe->new($eof_r, sub {}, undef);
+	PublicInbox::EOFpipe->new($eof_r, \&noop, undef);
 	my $sig = {
 		CHLD => \&PublicInbox::DS::enqueue_reap,
 		QUIT => $quit,
@@ -682,8 +666,21 @@ sub lazy_start {
 		}
 		$n; # true: continue, false: stop
 	});
+
+	# STDIN was redirected to /dev/null above, closing STDOUT and
+	# STDERR will cause the calling `lei' client process to finish
+	# reading <$daemon> pipe.
+	open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";
+	openlog($path, 'pid', 'user');
+	local $SIG{__WARN__} = sub { syslog('warning', "@_") };
+	my $owner_pid = $$;
+	my $on_destroy = PublicInbox::OnDestroy->new(sub {
+		syslog('crit', "$@") if $@ && $$ == $owner_pid;
+	});
+	open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
+	# $daemon pipe to `lei' closed, main loop begins:
 	PublicInbox::DS->EventLoop;
-	$@ = undef if $on_destroy; # quiet OnDestroy if we got here
+	@$on_destroy = (); # cancel on_destroy if we get here
 	exit($exit_code // 0);
 }
 
diff --git a/script/lei b/script/lei
index 2b041fb4..ceaf1e00 100755
--- a/script/lei
+++ b/script/lei
@@ -4,8 +4,8 @@
 use strict;
 use v5.10.1;
 use Socket qw(AF_UNIX SOCK_STREAM pack_sockaddr_un);
-
-if (eval { require IO::FDPass; 1 }) { # use daemon to reduce load time
+if (my ($sock, $pwd) = eval {
+	require IO::FDPass; # will try to use a daemon to reduce load time
 	my $path = do {
 		my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei';
 		if ($runtime_dir eq '/lei') {
@@ -21,32 +21,41 @@ if (eval { require IO::FDPass; 1 }) { # use daemon to reduce load time
 	my $addr = pack_sockaddr_un($path);
 	socket(my $sock, AF_UNIX, SOCK_STREAM, 0) or die "socket: $!";
 	unless (connect($sock, $addr)) { # start the daemon if not started
-		my $err = $! + 0;
-		my $env = { PERL5LIB => join(':', @INC) };
 		my $cmd = [ $^X, qw[-MPublicInbox::LEI
 			-E PublicInbox::LEI::lazy_start(@ARGV)],
-			$path, $err ];
+			$path, $! + 0 ];
+		my $env = { PERL5LIB => join(':', @INC) };
+		pipe(my ($daemon, $w)) or die "pipe: $!";
+		my $opt = { 1 => $w, 2 => $w };
 		require PublicInbox::Spawn;
-		waitpid(PublicInbox::Spawn::spawn($cmd, $env), 0);
-		warn "lei-daemon exited with \$?=$?\n" if $?;
+		my $pid = PublicInbox::Spawn::spawn($cmd, $env, $opt);
+		$opt = $w = undef;
+		while (<$daemon>) { warn $_ } # EOF when STDERR is redirected
+		waitpid($pid, 0) or warn <<"";
+lei-daemon could not start, PID:$pid exited with \$?=$?
 
 		# try connecting again anyways, unlink+bind may be racy
-		connect($sock, $addr) or die
-			"connect($path): $! (after attempted daemon start)";
+		unless (connect($sock, $addr)) {
+			die <<"";
+connect($path): $! (after attempted daemon start)
+Falling back to (slow) one-shot mode
+
+		}
 	}
 	require Cwd;
-	my $cwd = Cwd::fastcwd() // die "fastcwd: $!";
+	my $cwd = Cwd::fastcwd() // die "fastcwd(PWD=".($ENV{PWD}//'').": $!";
 	my $pwd = $ENV{PWD} // '';
-	if ($pwd eq $cwd) { # likely, all good
-	} elsif ($pwd) { # prefer ENV{PWD} if it's a symlink to real cwd
-		my @st_cwd = stat($cwd) or die "stat(cwd=$cwd): $!\n";
-		my @st_pwd = stat($pwd);
+	if ($pwd ne $cwd) { # prefer ENV{PWD} if it's a symlink to real cwd
+		my @st_cwd = stat($cwd) or die "stat(cwd=$cwd): $!";
+		my @st_pwd = stat($pwd); # PWD invalid, use cwd
 		# make sure st_dev/st_ino match for {PWD} to be valid
 		$pwd = $cwd if (!@st_pwd || $st_pwd[1] != $st_cwd[1] ||
 					$st_pwd[0] != $st_cwd[0]);
 	} else {
 		$pwd = $cwd;
 	}
+	($sock, $pwd);
+}) { # IO::FDPass, $sock, $pwd are all available:
 	local $ENV{PWD} = $pwd;
 	my $buf = "$$\0\0>" . join("]\0[", @ARGV) . "\0\0>";
 	while (my ($k, $v) = each %ENV) { $buf .= "$k=$v\0" }
@@ -60,6 +69,8 @@ if (eval { require IO::FDPass; 1 }) { # use daemon to reduce load time
 		die $buf;
 	}
 } else { # for systems lacking IO::FDPass
+	# don't warn about IO::FDPass since it's not commonly installed
+	warn $@ if $@ && index($@, 'IO::FDPass') < 0;
 	require PublicInbox::LEI;
 	PublicInbox::LEI::oneshot(__PACKAGE__);
 }
diff --git a/t/lei.t b/t/lei.t
index bdf6cc1c..cce90fff 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -127,7 +127,7 @@ my $test_lei_common = sub {
 my $test_lei_oneshot = $ENV{TEST_LEI_ONESHOT};
 SKIP: {
 	last SKIP if $test_lei_oneshot;
-	require_mods('IO::FDPass', 16);
+	require_mods(qw(IO::FDPass Cwd), 41);
 	my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/sock";
 
 	ok(run_script([qw(lei daemon-pid)], undef, $opt), 'daemon-pid');
@@ -188,7 +188,34 @@ SKIP: {
 	chomp(my $new_pid = $out);
 	ok(kill(0, $new_pid), 'new pid is running');
 	ok(-S $sock, 'sock exists again');
-	unlink $sock or BAIL_OUT "unlink $!";
+
+	if ('socket inaccessible') {
+		chmod 0000, $sock or BAIL_OUT "chmod 0000: $!";
+		$out = $err = '';
+		ok(run_script([qw(lei help)], undef, $opt),
+			'connect fail, one-shot fallback works');
+		like($err, qr/\bconnect\(/, 'connect error noted');
+		like($out, qr/^usage: /, 'help output works');
+		chmod 0700, $sock or BAIL_OUT "chmod 0700: $!";
+	}
+	if ('oneshot on cwd gone') {
+		my $cwd = Cwd::fastcwd() or BAIL_OUT "fastcwd: $!";
+		my $d = "$home/to-be-removed";
+		mkdir $d or BAIL_OUT "mkdir($d) $!";
+		chdir $d or BAIL_OUT "chdir($d) $!";
+		if (rmdir($d)) {
+			$out = $err = '';
+			ok(run_script([qw(lei help)], undef, $opt),
+				'cwd fail, one-shot fallback works');
+		} else {
+			$err = "rmdir=$!";
+		}
+		chdir $cwd or BAIL_OUT "chdir($cwd) $!";
+		like($err, qr/cwd\(/, 'cwd error noted');
+		like($out, qr/^usage: /, 'help output still works');
+	}
+
+	unlink $sock or BAIL_OUT "unlink($sock) $!";
 	for (0..100) {
 		kill('CHLD', $new_pid) or last;
 		tick();

  parent reply	other threads:[~2020-12-18 12:09 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18 12:09 [PATCH 00/26] lei: basic UI + IPC work Eric Wong
2020-12-18 12:09 ` [PATCH 01/26] lei: FD-passing and IPC basics Eric Wong
2020-12-18 12:09 ` [PATCH 02/26] lei: proposed command-listing and options Eric Wong
2021-02-18 20:42   ` lei q --save-as=... requires too much thinking Eric Wong
2020-12-18 12:09 ` [PATCH 03/26] lei_store: local storage for Local Email Interface Eric Wong
2020-12-18 12:09 ` [PATCH 04/26] tests: more common JSON module loading Eric Wong
2020-12-18 12:09 ` [PATCH 05/26] lei: use spawn (vfork + execve) for lazy start Eric Wong
2020-12-18 12:09 ` [PATCH 06/26] lei: refine help/option parsing, implement "init" Eric Wong
2020-12-18 12:09 ` [PATCH 07/26] t/lei-oneshot: standalone oneshot (non-socket) test Eric Wong
2020-12-18 12:09 ` [PATCH 08/26] lei: ensure we run a restrictive umask Eric Wong
2020-12-18 12:09 ` [PATCH 09/26] lei: support `daemon-env' for modifying long-lived env Eric Wong
2020-12-18 12:09 ` [PATCH 10/26] lei_store: simplify git_epoch_max, slightly Eric Wong
2020-12-18 12:09 ` [PATCH 11/26] search: simplify initialization, add ->xdb_shards_flat Eric Wong
2020-12-18 12:09 ` [PATCH 12/26] rename LeiDaemon package to PublicInbox::LEI Eric Wong
2020-12-18 12:09 ` [PATCH 13/26] lei: support pass-through for `lei config' Eric Wong
2020-12-18 12:09 ` [PATCH 14/26] lei: help: show actual paths being operated on Eric Wong
2020-12-18 12:09 ` [PATCH 15/26] lei: rename $client => $self and bless Eric Wong
2020-12-18 12:09 ` [PATCH 16/26] lei: micro-optimize startup time Eric Wong
2020-12-18 12:09 ` [PATCH 17/26] lei_store: relax GIT_COMMITTER_IDENT check Eric Wong
2020-12-18 12:09 ` [PATCH 18/26] lei_store: keyword extraction from mbox and Maildir Eric Wong
2020-12-18 12:09 ` [PATCH 19/26] on_destroy: generic localized END Eric Wong
2020-12-18 12:09 ` [PATCH 20/26] lei: restore default __DIE__ handler for event loop Eric Wong
2020-12-18 12:09 ` Eric Wong [this message]
2020-12-18 12:09 ` [PATCH 22/26] lei: start working on bash completion Eric Wong
2020-12-18 12:09 ` [PATCH 23/26] build: add lei.sh + "make symlink-install" target Eric Wong
2020-12-18 12:09 ` [PATCH 24/26] lei: support for -$DIGIT and -$SIG CLI switches Eric Wong
2020-12-18 12:09 ` [PATCH 25/26] lei: revise output routines Eric Wong
2020-12-18 12:09 ` [PATCH 26/26] lei: extinbox: start implementing in config file Eric Wong
2020-12-18 20:23   ` Eric Wong
2020-12-27 20:02   ` [PATCH 27/26] lei_xsearch: cross-(inbox|extindex) search 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=20201218120950.23272-22-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).