From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 173B51F934 for ; Tue, 28 Sep 2021 23:11:07 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 1/3] www: do not bump {over} refcnt on long responses Date: Tue, 28 Sep 2021 23:11:04 +0000 Message-Id: <20210928231106.5166-2-e@80x24.org> In-Reply-To: <20210928231106.5166-1-e@80x24.org> References: <20210928231106.5166-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: SQLite files may be replaced or removed by admins while generating a large threads or mailbox responses. Ensure we don't hold onto DBI handles and associated file descriptors past their cleanup. --- lib/PublicInbox/GzipFilter.pm | 6 ++++++ lib/PublicInbox/Mbox.pm | 29 ++++++++++++----------------- lib/PublicInbox/View.pm | 6 ++++-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm index c50c26c5..624c2ed3 100644 --- a/lib/PublicInbox/GzipFilter.pm +++ b/lib/PublicInbox/GzipFilter.pm @@ -84,6 +84,12 @@ sub gzip_or_die () { $gz; } +sub gone { # what: search/over/mm + my ($ctx, $what) = @_; + warn "W: `$ctx->{ibx}->{name}' $what went away unexpectedly\n"; + undef; +} + # for GetlineBody (via Qspawn) when NOT using $env->{'pi-httpd.async'} # Also used for ->getline callbacks sub translate ($$) { diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm index cec76182..dede4825 100644 --- a/lib/PublicInbox/Mbox.pm +++ b/lib/PublicInbox/Mbox.pm @@ -124,8 +124,9 @@ sub thread_cb { return $smsg; } # refill result set - $ctx->{msgs} = $msgs = $ctx->{over}->get_thread($ctx->{mid}, - $ctx->{prev}); + my $over = $ctx->{ibx}->over or return $ctx->gone('over'); + $ctx->{msgs} = $msgs = $over->get_thread($ctx->{mid}, + $ctx->{prev}); return unless @$msgs; $ctx->{prev} = $msgs->[-1]; } @@ -136,7 +137,6 @@ sub thread_mbox { my $msgs = $ctx->{msgs} = $over->get_thread($ctx->{mid}, {}); return [404, [qw(Content-Type text/plain)], []] if !@$msgs; $ctx->{prev} = $msgs->[-1]; - $ctx->{over} = $over; # bump refcnt require PublicInbox::MboxGz; PublicInbox::MboxGz::mbox_gz($ctx, \&thread_cb, $msgs->[0]->{subject}); } @@ -155,22 +155,23 @@ sub emit_range { sub all_ids_cb { my ($ctx) = @_; + my $over = $ctx->{ibx}->over or return $ctx->gone('over'); my $ids = $ctx->{ids}; do { while ((my $num = shift @$ids)) { - my $smsg = $ctx->{over}->get_art($num) or next; + my $smsg = $over->get_art($num) or next; return $smsg; } - $ctx->{ids} = $ids = $ctx->{over}->ids_after(\($ctx->{prev})); + $ctx->{ids} = $ids = $over->ids_after(\($ctx->{prev})); } while (@$ids); } sub mbox_all_ids { my ($ctx) = @_; my $prev = 0; - $ctx->{over} = $ctx->{ibx}->over or + my $over = $ctx->{ibx}->over or return PublicInbox::WWW::need($ctx, 'Overview'); - my $ids = $ctx->{over}->ids_after(\$prev) or return + my $ids = $over->ids_after(\$prev) or return [404, [qw(Content-Type text/plain)], ["No results found\n"]]; $ctx->{ids} = $ids; $ctx->{prev} = $prev; @@ -179,22 +180,16 @@ sub mbox_all_ids { PublicInbox::MboxGz::mbox_gz($ctx, \&all_ids_cb, 'all'); } -sub gone ($$) { - my ($ctx, $what) = @_; - warn "W: `$ctx->{ibx}->{inboxdir}' $what went away unexpectedly\n"; - undef; -} - sub results_cb { my ($ctx) = @_; - my $over = $ctx->{ibx}->over or return gone($ctx, 'over'); + my $over = $ctx->{ibx}->over or return $ctx->gone('over'); while (1) { while (defined(my $num = shift(@{$ctx->{ids}}))) { my $smsg = $over->get_art($num) or next; return $smsg; } # refill result set, deprioritize since there's many results - my $srch = $ctx->{ibx}->isrch or return gone($ctx, 'search'); + my $srch = $ctx->{ibx}->isrch or return $ctx->gone('search'); my $mset = $srch->mset($ctx->{query}, $ctx->{qopts}); my $size = $mset->size or return; $ctx->{qopts}->{offset} += $size; @@ -206,7 +201,7 @@ sub results_cb { sub results_thread_cb { my ($ctx) = @_; - my $over = $ctx->{ibx}->over or return gone($ctx, 'over'); + my $over = $ctx->{ibx}->over or return $ctx->gone('over'); while (1) { while (defined(my $num = shift(@{$ctx->{xids}}))) { my $smsg = $over->get_art($num) or next; @@ -217,7 +212,7 @@ sub results_thread_cb { next if $over->expand_thread($ctx); # refill result set, deprioritize since there's many results - my $srch = $ctx->{ibx}->isrch or return gone($ctx, 'search'); + my $srch = $ctx->{ibx}->isrch or return $ctx->gone('search'); my $mset = $srch->mset($ctx->{query}, $ctx->{qopts}); my $size = $mset->size or return; $ctx->{qopts}->{offset} += $size; diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm index 805e785b..069b9680 100644 --- a/lib/PublicInbox/View.pm +++ b/lib/PublicInbox/View.pm @@ -31,7 +31,9 @@ sub msg_page_i { my ($ctx, $eml) = @_; if ($eml) { # called by WwwStream::async_eml or getline my $smsg = $ctx->{smsg}; - $ctx->{smsg} = $ctx->{over}->next_by_mid(@{$ctx->{next_arg}}); + my $over = $ctx->{ibx}->over; + $ctx->{smsg} = $over ? $over->next_by_mid(@{$ctx->{next_arg}}) + : $ctx->gone('over'); $ctx->{mhref} = ($ctx->{nr} || $ctx->{smsg}) ? "../${\mid_href($smsg->{mid})}/" : ''; my $obuf = $ctx->{obuf} = _msg_page_prepare_obuf($eml, $ctx); @@ -70,7 +72,7 @@ sub msg_page { my ($ctx) = @_; my $ibx = $ctx->{ibx}; $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef; - my $over = $ctx->{over} = $ibx->over or return no_over_html($ctx); + my $over = $ibx->over or return no_over_html($ctx); my ($id, $prev); my $next_arg = $ctx->{next_arg} = [ $ctx->{mid}, \$id, \$prev ];