From d111c17ff06c9af7e0690e4b00a0eeb1c505c92a Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 21 Mar 2020 02:03:50 +0000 Subject: qspawn: handle ENOENT (and other errors on exec) As sqlite3(1) and other executables may become unavailable or uninstalled while a daemon runs, we need to gracefully handle errors in those cases. --- lib/PublicInbox/Qspawn.pm | 58 +++++++++++++++++++++++++++-------------------- t/httpd-corner.psgi | 7 ++++++ t/httpd-corner.t | 25 +++++++++++++++++++- 3 files changed, 64 insertions(+), 26 deletions(-) diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index 52aea3eb..34b6912f 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -45,7 +45,7 @@ sub new ($$$;) { sub _do_spawn { my ($self, $start_cb, $limiter) = @_; my $err; - my ($cmd, $cmd_env, $opt) = @{$self->{args}}; + my ($cmd, $cmd_env, $opt) = @{delete $self->{args}}; my %o = %{$opt || {}}; $self->{limiter} = $limiter; foreach my $k (PublicInbox::Spawn::RLIMITS()) { @@ -53,20 +53,17 @@ sub _do_spawn { $o{$k} = $rlimit; } } + $self->{cmd} = $o{quiet} ? undef : $cmd; eval { # popen_rd may die on EMFILE, ENFILE ($self->{rpipe}, $self->{pid}) = popen_rd($cmd, $cmd_env, \%o); - $self->{args} = $o{quiet} ? undef : $cmd; die "E: $!" unless defined($self->{pid}); $limiter->{running}++; $start_cb->($self); # EPOLL_CTL_ADD may ENOSPC/ENOMEM }; - if ($@) { - $self->{err} = $@; - finish($self); - } + finish($self, $@) if $@; } sub child_err ($) { @@ -83,16 +80,8 @@ sub log_err ($$) { $env->{'psgi.errors'}->print($msg, "\n"); } -# callback for dwaitpid -sub waitpid_err ($$) { - my ($self, $pid) = @_; - my $xpid = delete $self->{pid}; - my $err; - if ($pid > 0) { # success! - $err = child_err($?); - } elsif ($pid < 0) { # ??? does this happen in our case? - $err = "W: waitpid($xpid, 0) => $pid: $!"; - } # else should not be called with pid == 0 +sub finalize ($$) { + my ($self, $err) = @_; my ($env, $qx_cb, $qx_arg, $qx_buf) = delete @$self{qw(psgi_env qx_cb qx_arg qx_buf)}; @@ -108,16 +97,37 @@ sub waitpid_err ($$) { } if ($err) { - if ($self->{err}) { + if (defined $self->{err}) { $self->{err} .= "; $err"; } else { $self->{err} = $err; } - if ($env && $self->{args}) { - log_err($env, join(' ', @{$self->{args}}) . ": $err"); + if ($env && $self->{cmd}) { + log_err($env, join(' ', @{$self->{cmd}}) . ": $err"); } } - eval { $qx_cb->($qx_buf, $qx_arg) } if $qx_cb; + if ($qx_cb) { + eval { $qx_cb->($qx_buf, $qx_arg) }; + } elsif (my $wcb = delete $env->{'qspawn.wcb'}) { + # have we started writing, yet? + require PublicInbox::WwwStatic; + $wcb->(PublicInbox::WwwStatic::r(500)); + } +} + +# callback for dwaitpid +sub waitpid_err ($$) { + my ($self, $pid) = @_; + my $xpid = delete $self->{pid}; + my $err; + if (defined $pid) { + if ($pid > 0) { # success! + $err = child_err($?); + } elsif ($pid < 0) { # ??? does this happen in our case? + $err = "W: waitpid($xpid, 0) => $pid: $!"; + } # else should not be called with pid == 0 + } + finalize($self, $err); } sub do_waitpid ($) { @@ -133,14 +143,12 @@ sub do_waitpid ($) { } } -sub finish ($) { - my ($self) = @_; +sub finish ($;$) { + my ($self, $err) = @_; if (delete $self->{rpipe}) { do_waitpid($self); } else { - my ($env, $qx_cb, $qx_arg, $qx_buf) = - delete @$self{qw(psgi_env qx_cb qx_arg qx_buf)}; - eval { $qx_cb->($qx_buf, $qx_arg) } if $qx_cb; + finalize($self, $err); } } diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi index f2427234..44629620 100644 --- a/t/httpd-corner.psgi +++ b/t/httpd-corner.psgi @@ -94,6 +94,13 @@ my $app = sub { return $qsp->psgi_return($env, undef, sub { [ 200, [ qw(Content-Type application/octet-stream)]] }); + } elsif ($path eq '/psgi-return-enoent') { + require PublicInbox::Qspawn; + my $cmd = [ 'this-better-not-exist-in-PATH'.rand ]; + my $qsp = PublicInbox::Qspawn->new($cmd); + return $qsp->psgi_return($env, undef, sub { + [ 200, [ qw(Content-Type application/octet-stream)]] + }); } elsif ($path eq '/pid') { $code = 200; push @$body, "$$\n"; diff --git a/t/httpd-corner.t b/t/httpd-corner.t index e50aa436..cbfc8332 100644 --- a/t/httpd-corner.t +++ b/t/httpd-corner.t @@ -10,6 +10,7 @@ use PublicInbox::Spawn qw(which spawn); use PublicInbox::TestCommon; require_mods(qw(Plack::Util Plack::Builder HTTP::Date HTTP::Status)); use Digest::SHA qw(sha1_hex); +use IO::Handle (); use IO::Socket; use IO::Socket::UNIX; use Fcntl qw(:seek); @@ -335,6 +336,14 @@ SKIP: { is($out, "hello world\n"); } +{ + my $conn = conn_for($sock, 'psgi_return ENOENT'); + print $conn "GET /psgi-return-enoent HTTP/1.1\r\n\r\n" or die; + my $buf = ''; + sysread($conn, $buf, 16384, length($buf)) until $buf =~ /\r\n\r\n/; + like($buf, qr!HTTP/1\.[01] 500\b!, 'got 500 error on ENOENT'); +} + { my $conn = conn_for($sock, '1.1 pipeline together'); $conn->write("PUT /sha1 HTTP/1.1\r\nUser-agent: hello\r\n\r\n" . @@ -610,6 +619,11 @@ SKIP: { require_mods(@zmods, qw(Plack::Test HTTP::Request::Common), 3); use_ok 'HTTP::Request::Common'; use_ok 'Plack::Test'; + STDERR->flush; + open my $olderr, '>&', \*STDERR or die "dup stderr: $!"; + open my $tmperr, '+>', undef or die; + open STDERR, '>&', $tmperr or die; + STDERR->autoflush(1); my $app = require $psgi; test_psgi($app, sub { my ($cb) = @_; @@ -617,8 +631,17 @@ SKIP: { my $res = $cb->($req); my $buf = $res->content; IO::Uncompress::Gunzip::gunzip(\$buf => \(my $out)); - is($out, "hello world\n"); + is($out, "hello world\n", 'got expected output'); + + $req = GET('http://example.com/psgi-return-enoent'); + $res = $cb->($req); + is($res->code, 500, 'got error on ENOENT'); + seek($tmperr, 0, SEEK_SET) or die; + my $errbuf = do { local $/; <$tmperr> }; + like($errbuf, qr/this-better-not-exist/, + 'error logged about missing command'); }); + open STDERR, '>&', $olderr or die "restore stderr: $!"; } done_testing(); -- cgit v1.2.3-24-ge0c7