From 1d6e1f9a6a66a42d18f109aea406237cf8571597 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 26 May 2021 18:08:57 +0000 Subject: [PATCH] lei: require Socket::MsgHdr or Inline::C, drop oneshot The cost of supporting separate code paths between oneshot and daemon isn't worth the trouble; especially if there are more users to support. The test suite time nearly doubles with oneshot, so that's hurting developer productivity. FD passing is currently required to work efficiently with remote HTTP(S) queries which return large messages, as seen in commit 708b182a57373172f5523f3dc297659d58e03b58 ("ipc: wq: handle >MAX_ARG_STRLEN && {oneshot}) { # we'll die if disconnected: + if ($lei) { # we'll die if disconnected: pipe($out_r, my $out_w) or die "pipe: $!"; $lei->send_exec_cmd([ $in_r, $out_w ], $cmd, {}); } else { diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index c8d2f315b..6ff249d09 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -444,19 +444,6 @@ sub x_it ($$) { dump_and_clear_log(); if (my $s = $self->{pkt_op_p} // $self->{sock}) { send($s, "x_it $code", MSG_EOR); - } elsif ($self->{oneshot}) { - # don't want to end up using $? from child processes - _drop_wq($self); - # cleanup anything that has tempfiles or open file handles - %PATH2CFG = (); - delete @$self{qw(ovv dedupe sto cfg)}; - if (my $signum = ($code & 127)) { # usually SIGPIPE (13) - $SIG{PIPE} = 'DEFAULT'; # $SIG{$signum} doesn't work - kill $signum, $$; - sleep(1) while 1; # wait for signal - } else { - $quit->($code >> 8); - } } # else ignore if client disconnected } @@ -921,17 +908,6 @@ sub start_mua { my $io = []; $io->[0] = $self->{1} if $self->{opt}->{stdin} && -t $self->{1}; send_exec_cmd($self, $io, \@cmd, {}); - } elsif ($self->{oneshot}) { - my $pid = fork // die "fork: $!"; - if ($pid > 0) { # original process - if ($self->{opt}->{stdin} && -t STDOUT) { - open STDIN, '+<&', \*STDOUT or die "dup2: $!"; - } - exec(@cmd); - warn "exec @cmd: $!\n"; - POSIX::_exit(1); - } - POSIX::setsid() > 0 or die "setsid: $!"; } if ($self->{lxs} && $self->{au_done}) { # kick wait_startq syswrite($self->{au_done}, 'q' x ($self->{lxs}->{jobs} // 0)); @@ -952,14 +928,11 @@ sub send_exec_cmd { # tell script/lei to execute a command sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail my ($self) = @_; my $alerts = $self->{opt}->{alert} // return; + my $sock = $self->{sock}; while (my $op = shift(@$alerts)) { if ($op eq ':WINCH') { # hit the process group that started the MUA - if ($self->{sock}) { - send($self->{sock}, '-WINCH', MSG_EOR); - } elsif ($self->{oneshot}) { - kill('-WINCH', $$); - } + send($sock, '-WINCH', MSG_EOR) if $sock; } elsif ($op eq ':bell') { out($self, "\a"); } elsif ($op =~ /(?{sock}) { - send($s, exec_buf($cmd, {}), MSG_EOR); - } elsif ($self->{oneshot}) { - $self->{"pid.$self.$$"}->{spawn($cmd)} = $cmd; - } + send($sock, exec_buf($cmd, {}), MSG_EOR) if $sock; } else { err($self, "W: unsupported --alert=$op"); # non-fatal } @@ -1009,9 +978,6 @@ sub start_pager { if ($self->{sock}) { # lei(1) process runs it delete @$new_env{keys %$env}; # only set iff unset send_exec_cmd($self, [ @$rdr{0..2} ], [$pager], $new_env); - } elsif ($self->{oneshot}) { - my $cmd = [$pager]; - $self->{"pid.$self.$$"}->{spawn($cmd, $new_env, $rdr)} = $cmd; } else { die 'BUG: start_pager w/o socket'; } @@ -1253,29 +1219,13 @@ sub lazy_start { sub busy { 1 } # prevent daemon-shutdown if client is connected -# for users w/o Socket::Msghdr installed or Inline::C enabled -sub oneshot { - my ($main_pkg) = @_; - my $exit = $main_pkg->can('exit'); # caller may override exit() - local $quit = $exit if $exit; - local %PATH2CFG; - umask(077) // die("umask(077): $!"); - my $self = bless { oneshot => 1, env => \%ENV }, __PACKAGE__; - for (0..2) { open($self->{$_}, '+<&=', $_) or die "open fd=$_: $!" } - dispatch($self, @ARGV); - x_it($self, $self->{child_error}) if $self->{child_error}; -} - # ensures stdout hits the FS before sock disconnects so a client # can immediately reread it sub DESTROY { my ($self) = @_; $self->{1}->autoflush(1) if $self->{1}; stop_pager($self); - my $err = $?; - my $oneshot_pids = delete $self->{"pid.$self.$$"} or return; - waitpid($_, 0) for keys %$oneshot_pids; - $? = $err if $err; # preserve ->fail or ->x_it code + # preserve $? for ->fail or ->x_it code } sub wq_done_wait { # dwaitpid callback diff --git a/lib/PublicInbox/LeiEditSearch.pm b/lib/PublicInbox/LeiEditSearch.pm index 30ac65bdc..13713d245 100644 --- a/lib/PublicInbox/LeiEditSearch.pm +++ b/lib/PublicInbox/LeiEditSearch.pm @@ -14,19 +14,12 @@ sub lei_edit_search { my @cmd = (qw(git config --edit -f), $lss->{'-f'}); $lei->qerr("# spawning @cmd"); $lss->edit_begin($lei); - if ($lei->{oneshot}) { - require PublicInbox::Spawn; - waitpid(PublicInbox::Spawn::spawn(\@cmd), 0); - # non-fatal, editor could fail after successful write - $lei->child_error($?) if $?; - $lss->edit_done($lei); - } else { # run in script/lei foreground - require PublicInbox::PktOp; - my ($op_c, $op_p) = PublicInbox::PktOp->pair; - # $op_p will EOF when $EDITOR is done - $op_c->{ops} = { '' => [$lss->can('edit_done'), $lss, $lei] }; - $lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p ], \@cmd, {}); - } + # run in script/lei foreground + require PublicInbox::PktOp; + my ($op_c, $op_p) = PublicInbox::PktOp->pair; + # $op_p will EOF when $EDITOR is done + $op_c->{ops} = { '' => [$lss->can('edit_done'), $lss, $lei] }; + $lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p ], \@cmd, {}); } *_complete_edit_search = \&PublicInbox::LeiUp::_complete_up; diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm index a7a0ebef0..af5edbc24 100644 --- a/lib/PublicInbox/LeiStore.pm +++ b/lib/PublicInbox/LeiStore.pm @@ -431,20 +431,15 @@ sub write_prepare { my $d = $lei->store_path; $self->ipc_lock_init("$d/ipc.lock"); substr($d, -length('/lei/store'), 10, ''); - my $err_pipe; - unless ($lei->{oneshot}) { - pipe(my ($r, $w)) or die "pipe: $!"; - $err_pipe = [ $r, $w ]; - } + pipe(my ($r, $w)) or die "pipe: $!"; + my $err_pipe = [ $r, $w ]; # Mail we import into lei are private, so headers filtered out # by -mda for public mail are not appropriate local @PublicInbox::MDA::BAD_HEADERS = (); $self->ipc_worker_spawn("lei/store $d", $lei->oldset, { lei => $lei, err_pipe => $err_pipe }); - if ($err_pipe) { - require PublicInbox::LeiStoreErr; - PublicInbox::LeiStoreErr->new($err_pipe->[0], $lei); - } + require PublicInbox::LeiStoreErr; + PublicInbox::LeiStoreErr->new($err_pipe->[0], $lei); } $lei->{sto} = $self; } diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm index 2ce64d629..a71158f36 100644 --- a/lib/PublicInbox/LeiSucks.pm +++ b/lib/PublicInbox/LeiSucks.pm @@ -23,8 +23,7 @@ sub lei_sucks { } eval { require PublicInbox }; my $pi_ver = eval('$PublicInbox::VERSION') // '(???)'; - my $daemon = $lei->{oneshot} ? 'oneshot' : 'daemon'; - my @out = ("lei $pi_ver mode=$daemon\n", + my @out = ("lei $pi_ver\n", "perl $Config{version} / $os $rel / $mac ". "ptrsize=$Config{ptrsize}\n"); chomp(my $gv = `git --version` || "git missing"); diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm index 4399c4fba..9069232b4 100644 --- a/lib/PublicInbox/LeiUp.pm +++ b/lib/PublicInbox/LeiUp.pm @@ -76,22 +76,18 @@ sub lei_up { my @all = PublicInbox::LeiSavedSearch::list($lei); my @local = grep(!m!\Aimaps?://!i, @all); $lei->_lei_store->write_prepare($lei); # share early - if ($lei->{oneshot}) { # synchronous - up1_redispatch($lei, $_) for @local; - } else { - # daemon mode, re-dispatch into our event loop w/o - # creating an extra fork-level - require PublicInbox::DS; - require PublicInbox::PktOp; - my ($op_c, $op_p) = PublicInbox::PktOp->pair; - for my $o (@local) { - PublicInbox::DS::requeue(sub { - up1_redispatch($lei, $o, $op_p); - }); - } - $lei->event_step_init; - $op_c->{ops} = { '' => [$lei->can('dclose'), $lei] }; + # daemon mode, re-dispatch into our event loop w/o + # creating an extra fork-level + require PublicInbox::DS; + require PublicInbox::PktOp; + my ($op_c, $op_p) = PublicInbox::PktOp->pair; + for my $o (@local) { + PublicInbox::DS::requeue(sub { + up1_redispatch($lei, $o, $op_p); + }); } + $lei->event_step_init; + $op_c->{ops} = { '' => [$lei->can('dclose'), $lei] }; } else { up1($lei, $out); } diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 460c9da01..83dcf650d 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -544,7 +544,8 @@ EOM ($tmpdir, $for_destroy) = tmpdir unless $tmpdir; state $persist_xrd = $ENV{TEST_LEI_DAEMON_PERSIST_DIR}; SKIP: { - skip 'TEST_LEI_ONESHOT set', 1 if $ENV{TEST_LEI_ONESHOT}; + $ENV{TEST_LEI_ONESHOT} and + xbail 'TEST_LEI_ONESHOT no longer supported'; my $home = "$tmpdir/lei-daemon"; mkdir($home, 0700) or BAIL_OUT "mkdir: $!"; local $ENV{HOME} = $home; @@ -568,22 +569,12 @@ EOM lei_ok(qw(daemon-kill), \"daemon-kill after $t"); } }; # SKIP for lei_daemon - unless ($test_opt->{daemon_only}) { - $ENV{TEST_LEI_DAEMON_ONLY} and - skip 'TEST_LEI_DAEMON_ONLY set', 1; - require_ok 'PublicInbox::LEI'; - my $home = "$tmpdir/lei-oneshot"; - mkdir($home, 0700) or BAIL_OUT "mkdir: $!"; - local $ENV{HOME} = $home; - local $ENV{XDG_RUNTIME_DIR} = '/dev/null'; - $cb->(); - } if ($daemon_pid) { for (0..10) { kill(0, $daemon_pid) or last; tick; } - ok(!kill(0, $daemon_pid), "$t daemon stopped after oneshot"); + ok(!kill(0, $daemon_pid), "$t daemon stopped"); my $f = "$daemon_xrd/lei/errors.log"; open my $fh, '<', $f or BAIL_OUT "$f: $!"; my @l = <$fh>; diff --git a/script/lei b/script/lei index bec6b0012..4d752fd8a 100755 --- a/script/lei +++ b/script/lei @@ -9,10 +9,18 @@ my $narg = 5; my $sock; my $recv_cmd = PublicInbox::CmdIPC4->can('recv_cmd4'); my $send_cmd = PublicInbox::CmdIPC4->can('send_cmd4') // do { + my $inline_dir = $ENV{PERL_INLINE_DIRECTORY} //= ( + $ENV{XDG_CACHE_HOME} // + ( ($ENV{HOME} // '/nonexistent').'/.cache' ) + ).'/public-inbox/inline-c'; + if (!-d $inline_dir) { + require File::Path; + File::Path::make_path($inline_dir); + } require PublicInbox::Spawn; # takes ~50ms even if built *sigh* $recv_cmd = PublicInbox::Spawn->can('recv_cmd4'); PublicInbox::Spawn->can('send_cmd4'); -}; +} // die 'please install Inline::C or Socket::MsgHdr'; my %pids; my $sigchld = sub { @@ -66,80 +74,69 @@ my $exec_cmd = sub { } }; -if ($send_cmd && eval { - my $path = do { - my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei'; - die \0 if $runtime_dir eq '/dev/null/lei'; # oneshot forced - if ($runtime_dir eq '/lei') { - require File::Spec; - $runtime_dir = File::Spec->tmpdir."/lei-$<"; - } - unless (-d $runtime_dir) { - require File::Path; - File::Path::mkpath($runtime_dir, 0, 0700); - } - "$runtime_dir/$narg.seq.sock"; - }; - my $addr = pack_sockaddr_un($path); - socket($sock, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!"; - unless (connect($sock, $addr)) { # start the daemon if not started - local $ENV{PERL5LIB} = join(':', @INC); - open(my $daemon, '-|', $^X, qw[-MPublicInbox::LEI - -E PublicInbox::LEI::lazy_start(@ARGV)], - $path, $! + 0, $narg) or die "popen: $!"; - while (<$daemon>) { warn $_ } # EOF when STDERR is redirected - close($daemon) or warn <<""; +my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei'; +if ($runtime_dir eq '/lei') { + require File::Spec; + $runtime_dir = File::Spec->tmpdir."/lei-$<"; +} +unless (-d $runtime_dir) { + require File::Path; + File::Path::make_path($runtime_dir, { mode => 0700 }); +} +my $path = "$runtime_dir/$narg.seq.sock"; +my $addr = pack_sockaddr_un($path); +socket($sock, AF_UNIX, SOCK_SEQPACKET, 0) or die "socket: $!"; +unless (connect($sock, $addr)) { # start the daemon if not started + local $ENV{PERL5LIB} = join(':', @INC); + open(my $daemon, '-|', $^X, qw[-MPublicInbox::LEI + -E PublicInbox::LEI::lazy_start(@ARGV)], + $path, $! + 0, $narg) or die "popen: $!"; + while (<$daemon>) { warn $_ } # EOF when STDERR is redirected + close($daemon) or warn <<""; lei-daemon could not start, exited with \$?=$? - # try connecting again anyways, unlink+bind may be racy - connect($sock, $addr) or die <<""; + # try connecting again anyways, unlink+bind may be racy + connect($sock, $addr) or die <<""; connect($path): $! (after attempted daemon start) Falling back to (slow) one-shot mode +} +# (Socket::MsgHdr|Inline::C), $sock are all available: +open my $dh, '<', '.' or die "open(.) $!"; +my $buf = join("\0", scalar(@ARGV), @ARGV); +while (my ($k, $v) = each %ENV) { $buf .= "\0$k=$v" } +$buf .= "\0\0"; +my $n = $send_cmd->($sock, [0, 1, 2, fileno($dh)], $buf, MSG_EOR); +if (!$n) { + die "sendmsg: $! (check RLIMIT_NOFILE)\n" if $!{ETOOMANYREFS}; + die "sendmsg: $!\n"; +} +my $x_it_code = 0; +while (1) { + my (@fds) = $recv_cmd->($sock, my $buf, 4096 * 33); + if (scalar(@fds) == 1 && !defined($fds[0])) { + next if $!{EINTR}; + last if $!{ECONNRESET}; + die "recvmsg: $!"; } - # (Socket::MsgHdr|Inline::C), $sock are all available: - open my $dh, '<', '.' or die "open(.) $!"; - my $buf = join("\0", scalar(@ARGV), @ARGV); - while (my ($k, $v) = each %ENV) { $buf .= "\0$k=$v" } - $buf .= "\0\0"; - my $n = $send_cmd->($sock, [0, 1, 2, fileno($dh)], $buf, MSG_EOR); - if (!$n) { - die "sendmsg: $! (check RLIMIT_NOFILE)\n" if $!{ETOOMANYREFS}; - die "sendmsg: $!\n"; - } - 1; -}) { # connected and request sent to lei-daemon, wait for responses or EOF - my $x_it_code = 0; - while (1) { - my (@fds) = $recv_cmd->($sock, my $buf, 4096 * 33); - if (scalar(@fds) == 1 && !defined($fds[0])) { - next if $!{EINTR}; - last if $!{ECONNRESET}; - die "recvmsg: $!"; - } - last if $buf eq ''; - if ($buf =~ /\Aexec (.+)\z/) { - $exec_cmd->(\@fds, split(/\0/, $1)); - } elsif ($buf eq '-WINCH') { - kill($buf, @parent); # for MUA - } elsif ($buf =~ /\Ax_it ([0-9]+)\z/) { - $x_it_code ||= $1 + 0; - last; - } elsif ($buf =~ /\Achild_error ([0-9]+)\z/) { - $x_it_code ||= $1 + 0; - } else { - $sigchld->(); - die $buf; - } - } - $sigchld->(); - if (my $sig = ($x_it_code & 127)) { - kill $sig, $$; - sleep(1) while 1; + last if $buf eq ''; + if ($buf =~ /\Aexec (.+)\z/) { + $exec_cmd->(\@fds, split(/\0/, $1)); + } elsif ($buf eq '-WINCH') { + kill($buf, @parent); # for MUA + } elsif ($buf =~ /\Ax_it ([0-9]+)\z/) { + $x_it_code ||= $1 + 0; + last; + } elsif ($buf =~ /\Achild_error ([0-9]+)\z/) { + $x_it_code ||= $1 + 0; + } else { + $sigchld->(); + die $buf; } - exit($x_it_code >> 8); -} else { # for systems lacking Socket::MsgHdr or Inline::C - warn $@ if $@ && !ref($@); - require PublicInbox::LEI; - PublicInbox::LEI::oneshot(__PACKAGE__); } +$sigchld->(); +if (my $sig = ($x_it_code & 127)) { + kill $sig, $$; + sleep(1) while 1; +} +exit($x_it_code >> 8); diff --git a/t/lei-daemon.t b/t/lei-daemon.t index 84e2791d6..a7c4b7992 100644 --- a/t/lei-daemon.t +++ b/t/lei-daemon.t @@ -73,15 +73,6 @@ test_lei({ daemon_only => 1 }, sub { lei_ok('daemon-pid'); chomp $lei_out; is($lei_out, $new_pid, 'PID unchanged after -0/-CHLD'); - - SKIP: { # socket inaccessible - skip "cannot test connect EPERM as root", 3 if $> == 0; - chmod 0000, $sock or BAIL_OUT "chmod 0000: $!"; - lei_ok('help', \'connect fail, one-shot fallback works'); - like($lei_err, qr/\bconnect\(/, 'connect error noted'); - like($lei_out, qr/^usage: /, 'help output works'); - chmod 0700, $sock or BAIL_OUT "chmod 0700: $!"; - } unlink $sock or BAIL_OUT "unlink($sock) $!"; for (0..100) { kill('CHLD', $new_pid) or last; -- 2.39.5