1/4 avoids some log noise due to expected occurences. The others are small cleanups and doc improvements noticed while doing other WIP stuff... 3/4 ought to make things easier for my puny brain :x Eric Wong (4): www: gzip_filter: gracefully handle socket ->write failures www: gzip_filter: update a few comments ds: use ->dflush to distinguish from ->zflush www: simplify GzipFilter->zflush callers lib/PublicInbox/DS.pm | 4 ++-- lib/PublicInbox/DSdeflate.pm | 4 ++-- lib/PublicInbox/GzipFilter.pm | 10 ++++++---- lib/PublicInbox/IMAP.pm | 2 +- lib/PublicInbox/Mbox.pm | 3 +-- lib/PublicInbox/NNTP.pm | 2 +- lib/PublicInbox/WwwText.pm | 6 +----- 7 files changed, 14 insertions(+), 17 deletions(-)
Socket ->write failures are expected and common for TCP traffic, especially if it's facing unreliable remote connections. So just bail out silently if our {gz} field was already clobbered during the small bit of recursion we hit on ->write failures from async responses. This ought to fix some GzipFilter::zflush errors (via $forward ->close from PublicInbox::HTTP) I've been noticing on deployments running -netd. I'm still unsure as to why I hadn't seen them before, but it might've only been ignorance on my part... Link: https://public-inbox.org/meta/20220802065436.GA13935@dcvr/ --- lib/PublicInbox/GzipFilter.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm index e37f1f76..c586d2f8 100644 --- a/lib/PublicInbox/GzipFilter.pm +++ b/lib/PublicInbox/GzipFilter.pm @@ -149,10 +149,11 @@ sub zflush ($;$) { my $zbuf = delete $self->{zbuf}; my $gz = delete $self->{gz}; my $err; - if (defined $_[1]) { + if (defined $_[1]) { # it's a bug iff $gz is undef w/ $_[1] $err = $gz->deflate($_[1], $zbuf); die "gzip->deflate: $err" if $err != Z_OK; } + $gz // return; # not a bug, recursing on DS->write failure $err = $gz->flush($zbuf); die "gzip->flush: $err" if $err != Z_OK; $zbuf;
A few things I noticed while reviewing and evaluating the PSGI code for JMAP support. --- lib/PublicInbox/GzipFilter.pm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm index c586d2f8..d41748c4 100644 --- a/lib/PublicInbox/GzipFilter.pm +++ b/lib/PublicInbox/GzipFilter.pm @@ -1,4 +1,4 @@ -# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org> +# Copyright (C) all contributors <meta@public-inbox.org> # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt> # # In public-inbox <=1.5.0, public-inbox-httpd favored "getline" @@ -116,6 +116,7 @@ sub translate ($$) { } } +# returns PublicInbox::HTTP::{Chunked,Identity} sub http_out ($) { my ($self) = @_; $self->{http_out} // do { @@ -183,7 +184,7 @@ sub bail { # this is public-inbox-httpd-specific sub async_blob_cb { # git->cat_async callback my ($bref, $oid, $type, $size, $self) = @_; - my $http = $self->{env}->{'psgix.io'}; + my $http = $self->{env}->{'psgix.io'}; # PublicInbox::HTTP $http->{forward} or return; # client aborted my $smsg = $self->{smsg} or bail($self, 'BUG: no smsg'); if (!defined($oid)) { @@ -195,7 +196,7 @@ sub async_blob_cb { # git->cat_async callback $smsg->{blob} eq $oid or bail($self, "BUG: $smsg->{blob} != $oid"); eval { $self->async_eml(PublicInbox::Eml->new($bref)) }; bail($self, "E: async_eml: $@") if $@; - if ($self->{-low_prio}) { + if ($self->{-low_prio}) { # run via PublicInbox::WWW::event_step push(@{$self->{www}->{-low_prio_q}}, $self) == 1 and PublicInbox::DS::requeue($self->{www}); } else {
->zflush is already for GzipFilter in PublicInbox::WWW, while we use DEFLATE for NNTP and IMAP. This ought to make the code easier-to-follow. --- lib/PublicInbox/DS.pm | 4 ++-- lib/PublicInbox/DSdeflate.pm | 4 ++-- lib/PublicInbox/IMAP.pm | 2 +- lib/PublicInbox/NNTP.pm | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index ef483aac..77e2e5e9 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -648,7 +648,7 @@ sub shutdn ($) { } } -sub zflush {} # overridden by DSdeflate +sub dflush {} # overridden by DSdeflate sub compressed {} # overridden by DSdeflate sub long_response_done {} # overridden by Net::NNTP @@ -682,7 +682,7 @@ sub requeue_once { my ($self) = @_; # COMPRESS users all share the same DEFLATE context. # Flush it here to ensure clients don't see each other's data - $self->zflush; + $self->dflush; # no recursion, schedule another call ASAP, # but only after all pending writes are done. diff --git a/lib/PublicInbox/DSdeflate.pm b/lib/PublicInbox/DSdeflate.pm index bf0ecd38..639690e2 100644 --- a/lib/PublicInbox/DSdeflate.pm +++ b/lib/PublicInbox/DSdeflate.pm @@ -30,7 +30,7 @@ my %IN_OPT = ( my ($zout, $zbuf); { my $err; - $zbuf = \(my $initial = ''); # replaced by $next in zflush/write + $zbuf = \(my $initial = ''); # replaced by $next in dflush/write ($zout, $err) = Compress::Raw::Zlib::Deflate->new( # nnrpd (INN) and Compress::Raw::Zlib favor MemLevel=9, # the zlib C library and git use MemLevel=8 as the default @@ -100,7 +100,7 @@ sub msg_more ($$) { 1; } -sub zflush ($) { +sub dflush ($) { my ($self) = @_; my $deflated = $zbuf; diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm index 805f1102..0f0f9b3a 100644 --- a/lib/PublicInbox/IMAP.pm +++ b/lib/PublicInbox/IMAP.pm @@ -595,7 +595,7 @@ sub fetch_blob_cb { # called by git->cat_async via ibx_async_cat \&fetch_blob_cb, $fetch_arg); } fetch_run_ops($self, $smsg, $bref, $ops, $partial); - $pre ? $self->zflush : $self->requeue_once; + $pre ? $self->dflush : $self->requeue_once; } sub emit_rfc822 { diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index b223eb07..791fe2a9 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -556,7 +556,7 @@ sub blob_cb { # called by git->cat_async via ibx_async_cat $self->close; die "BUG: bad code: $r"; } - $self->write(\".\r\n"); # flushes (includes ->zflush) + $self->write(\".\r\n"); # flushes (includes ->dflush) $self->requeue; }
->zflush can take a buffer arg, so there's no need to make a separate call to ->translate in some cases. --- lib/PublicInbox/Mbox.pm | 3 +-- lib/PublicInbox/WwwText.pm | 6 +----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm index b977308d..e65f38f0 100644 --- a/lib/PublicInbox/Mbox.pm +++ b/lib/PublicInbox/Mbox.pm @@ -24,8 +24,7 @@ sub getline { if ($n) { $ctx->translate(msg_body($eml)); } else { # last message - $ctx->zmore(msg_body($eml)); - $ctx->zflush; + $ctx->zflush(msg_body($eml)); } } diff --git a/lib/PublicInbox/WwwText.pm b/lib/PublicInbox/WwwText.pm index 2937c333..369328ee 100644 --- a/lib/PublicInbox/WwwText.pm +++ b/lib/PublicInbox/WwwText.pm @@ -38,11 +38,7 @@ sub get_text { } my $env = $ctx->{env}; if ($raw) { - if ($code == 200) { - my $gzf = gzf_maybe($hdr, $env); - $txt = $gzf->translate($txt); - $txt .= $gzf->zflush; - } + $txt = gzf_maybe($hdr, $env)->zflush($txt) if $code == 200; $hdr->[3] = length($txt); return [ $code, $hdr, [ $txt ] ] }