From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 9E5761FA52 for ; Wed, 25 Oct 2023 00:29:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1698193792; bh=RWWuKm70NFkDTjbV1/56hmNMyp7PdYlsHb3sx8dSmgM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=3p22BeHgQ+i24QiqEzkfq4ww8EW2wTE63zPIVLrDLQJZjlc0fZM4JiHzT7xqzVHS1 vA1d3x7iRbsu2BIkZ9vseHDIY0VV01eRh1Q0GetpOxlpNN1IdGrWR+djSJJ2On55Xf BGJRK3tgtxzZ+t18MxtHZtycG8AoESh7fuDc5ABQ= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 18/26] qspawn: simplify internal argument passing Date: Wed, 25 Oct 2023 00:29:41 +0000 Message-ID: <20231025002949.3092193-19-e@80x24.org> In-Reply-To: <20231025002949.3092193-1-e@80x24.org> References: <20231025002949.3092193-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Now that psgi_return is gone, we can further simplify our internals to support only psgi_qx and psgi_yield. Internal argument passing is reduced and we keep the command env and redirects in the Qspawn object for as long as it's alive. I wanted to get rid of finalize() entirely, but it seems trickier to do when having to support generic PSGI. --- lib/PublicInbox/Qspawn.pm | 50 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index 0bb02081..a03e1b01 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -47,28 +47,27 @@ my $def_limiter; # {qsp_err} is an optional error buffer callers may access themselves sub new { my ($class, $cmd, $cmd_env, $opt) = @_; - bless { args => [ $cmd, $cmd_env, $opt ] }, $class; + bless { args => [ $cmd, $cmd_env, $opt ? { %$opt } : {} ] }, $class; } sub _do_spawn { my ($self, $start_cb, $limiter) = @_; - my ($cmd, $cmd_env, $opt) = @{delete $self->{args}}; + my ($cmd, $cmd_env, $opt) = @{$self->{args}}; my %o = %{$opt || {}}; $self->{limiter} = $limiter; for my $k (@PublicInbox::Spawn::RLIMITS) { - $o{$k} = $limiter->{$k} // next; + $opt->{$k} = $limiter->{$k} // next; } - $self->{cmd} = $cmd; $self->{-quiet} = 1 if $o{quiet}; $limiter->{running}++; if ($start_cb) { eval { # popen_rd may die on EMFILE, ENFILE - $self->{rpipe} = popen_rd($cmd, $cmd_env, \%o, - \&waitpid_err, $self, \%o); + $self->{rpipe} = popen_rd($cmd, $cmd_env, $opt, + \&waitpid_err, $self); $start_cb->($self); # EPOLL_CTL_ADD may ENOSPC/ENOMEM }; } else { - eval { run_await($cmd, $cmd_env, \%o, \&wait_await, $self) }; + eval { run_await($cmd, $cmd_env, $opt, \&wait_await, $self) }; warn "E: $@" if $@; } finish($self, $@) if $@; @@ -79,8 +78,8 @@ sub psgi_status_err { # Qspawn itself is useful w/o PSGI PublicInbox::WwwStatic::r($_[0] // 500); } -sub finalize ($;$) { - my ($self, $opt) = @_; +sub finalize ($) { + my ($self) = @_; # process is done, spawn whatever's in the queue my $limiter = delete $self->{limiter} or return; @@ -96,13 +95,13 @@ sub finalize ($;$) { if (my $dst = $self->{qsp_err}) { $$dst .= $$dst ? " $err" : "; $err"; } - warn "@{$self->{cmd}}: $err\n" if !$self->{-quiet}; + warn "E: @{$self->{args}->[0]}: $err\n" if !$self->{-quiet}; } my ($env, $qx_cb_arg) = delete @$self{qw(psgi_env qx_cb_arg)}; if ($qx_cb_arg) { my $cb = shift @$qx_cb_arg; - eval { $cb->($opt->{1}, @$qx_cb_arg) }; + eval { $cb->($self->{args}->[2]->{1}, @$qx_cb_arg) }; return unless $@; warn "E: $@"; # hope qspawn.wcb can handle it } @@ -113,23 +112,21 @@ sub finalize ($;$) { } } -sub DESTROY { finalize($_[0]) } # ->finalize is idempotent - sub waitpid_err { # callback for awaitpid - my (undef, $self, $opt) = @_; # $_[0]: pid + my (undef, $self) = @_; # $_[0]: pid $self->{_err} = ''; # for defined check in ->finish - if ($?) { # FIXME: redundant + if ($?) { # XXX this may be redundant my $status = $? >> 8; my $sig = $? & 127; $self->{_err} .= "exit status=$status"; $self->{_err} .= " signal=$sig" if $sig; } - finalize($self, $opt) if !$self->{rpipe}; + finalize($self) if !$self->{rpipe}; } sub wait_await { # run_await cb my ($pid, $cmd, $cmd_env, $opt, $self) = @_; - waitpid_err($pid, $self, $opt); + waitpid_err($pid, $self); } sub yield_chunk { # $_[-1] is sysread buffer (or undef) @@ -214,26 +211,24 @@ sub yield_pass { sub parse_hdr_done ($$) { my ($self) = @_; - my $ret; + my ($ret, $err); if (defined $_[-1]) { my ($bref, $ph_cb, @ph_arg) = @{$self->{yield_parse_hdr}}; $$bref .= $_[-1]; $ret = eval { $ph_cb->(length($_[-1]), $bref, @ph_arg) }; - if ($@) { - carp "parse_hdr (@{$self->{cmd}}): $@\n"; + if (($err = $@)) { $ret = psgi_status_err(); } elsif (!$ret && $_[-1] eq '') { - carp <{cmd}} ($self->{psgi_env}->{REQUEST_URI}) -EOM + $err = 'EOF'; $ret = psgi_status_err(); } } else { - carp <{cmd}} ($self->{psgi_env}->{REQUEST_URI}) -EOM + $err = "$!"; $ret = psgi_status_err(); } + carp <{args}->[0]} ($self->{psgi_env}->{REQUEST_URI}) +EOM $ret; # undef if headers incomplete } @@ -301,4 +296,7 @@ sub psgi_yield { } } +no warnings 'once'; +*DESTROY = \&finalize; # ->finalize is idempotent + 1;