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 08AE61F8C6; Tue, 24 Aug 2021 22:49:25 +0000 (UTC) Date: Tue, 24 Aug 2021 22:49:24 +0000 From: Eric Wong To: Konstantin Ryabitsev Cc: meta@public-inbox.org Subject: [PATCH] imap+nntp: die loudly if ->mm or ->over disappear Message-ID: <20210824224924.GA29409@dcvr> References: <20210824135835.7ujvhisruasihlpo@nitro.local> <20210824201115.GA8587@dcvr> <20210824204855.ejspej4z7r2rpu63@nitro.local> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210824204855.ejspej4z7r2rpu63@nitro.local> List-Id: Konstantin Ryabitsev wrote: > Cleaning up the bogus entry in PI_CONFIG makes everything work just fine, but > you may want to catch this condition early, as it may happen with mirroring Agreed, I think the following should catch most cases: ----------8<--------- Subject: [PATCH] imap+nntp: die loudly if ->mm or ->over disappear While the WWW front-end can gracefully handle ->mm and ->over disappearing (in most cases), IMAP+NNTP front-ends are completely dependent on these and failed mysteriously when they go missing after startup. These will hopefully make issues like what Konstantin encountered more obvious: Link: https://public-inbox.org/meta/20210824204855.ejspej4z7r2rpu63@nitro.local/ --- lib/PublicInbox/IMAP.pm | 25 +++++++++--------- lib/PublicInbox/Inbox.pm | 19 +++++++------- lib/PublicInbox/NNTP.pm | 56 ++++++++++++++++++++-------------------- 3 files changed, 51 insertions(+), 49 deletions(-) diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm index 9402aa41..37e07dae 100644 --- a/lib/PublicInbox/IMAP.pm +++ b/lib/PublicInbox/IMAP.pm @@ -195,14 +195,14 @@ sub cmd_capability ($$) { # but uo2m_hibernate can compact and deduplicate it sub uo2m_ary_new ($;$) { my ($self, $exists) = @_; - my $base = $self->{uid_base}; - my $uids = $self->{ibx}->over->uid_range($base + 1, $base + UID_SLICE); + my $ub = $self->{uid_base}; + my $uids = $self->{ibx}->over(1)->uid_range($ub + 1, $ub + UID_SLICE); # convert UIDs to offsets from {base} my @tmp; # [$UID_OFFSET] => $MSN my $msn = 0; - ++$base; - $tmp[$_ - $base] = ++$msn for @$uids; + ++$ub; + $tmp[$_ - $ub] = ++$msn for @$uids; $$exists = $msn if $exists; \@tmp; } @@ -243,7 +243,7 @@ sub uo2m_extend ($$;$) { # need to extend the current range: my $base = $self->{uid_base}; ++$beg; - my $uids = $self->{ibx}->over->uid_range($beg, $base + UID_SLICE); + my $uids = $self->{ibx}->over(1)->uid_range($beg, $base + UID_SLICE); return $uo2m if !scalar(@$uids); my @tmp; # [$UID_OFFSET] => $MSN my $write_method = $_[2] // 'msg_more'; @@ -342,7 +342,7 @@ sub cmd_idle ($$) { my $fd = fileno($sock); $self->{-idle_tag} = $tag; # only do inotify on most recent slice - if ($ibx->over->max < $uid_end) { + if ($ibx->over(1)->max < $uid_end) { $ibx->subscribe_unlock($fd, $self); $self->{imapd}->idler_start; } @@ -393,7 +393,7 @@ sub inbox_lookup ($$;$) { my ($ibx, $exists, $uidmax, $uid_base) = (undef, 0, 0, 0); $mailbox = lc $mailbox; $ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return; - my $over = $ibx->over; + my $over = $ibx->over(1); if ($over != $ibx) { # not a dummy $mailbox =~ /\.([0-9]+)\z/ or die "BUG: unexpected dummy mailbox: $mailbox\n"; @@ -418,7 +418,8 @@ sub inbox_lookup ($$;$) { # if "INBOX.foo.bar" is selected and "INBOX.foo.bar.0", # check for new UID ranges (e.g. "INBOX.foo.bar.1") if (my $z = $self->{imapd}->{mailboxes}->{"$mailbox.0"}) { - ensure_slices_exist($self->{imapd}, $z, $z->over->max); + ensure_slices_exist($self->{imapd}, $z, + $z->over(1)->max); } } ($ibx, $exists, $uidmax + 1, $uid_base); @@ -722,7 +723,7 @@ sub range_step ($$) { uid_clamp($self, \$beg, \$end); } elsif ($range =~ /\A([0-9]+):\*\z/) { $beg = $1 + 0; - $end = $self->{ibx}->over->max; + $end = $self->{ibx}->over(1)->max; $end = $uid_end if $end > $uid_end; $beg = $end if $beg > $end; uid_clamp($self, \$beg, \$end); @@ -740,7 +741,7 @@ sub range_step ($$) { sub refill_range ($$$) { my ($self, $msgs, $range_info) = @_; my ($beg, $end, $range_csv) = @$range_info; - if (scalar(@$msgs = @{$self->{ibx}->over->query_xover($beg, $end)})) { + if (scalar(@$msgs = @{$self->{ibx}->over(1)->query_xover($beg, $end)})){ $range_info->[0] = $msgs->[-1]->{num} + 1; return; } @@ -781,7 +782,7 @@ sub fetch_smsg { # long_response sub refill_uids ($$$;$) { my ($self, $uids, $range_info, $sql) = @_; my ($beg, $end, $range_csv) = @$range_info; - my $over = $self->{ibx}->over; + my $over = $self->{ibx}->over(1); while (1) { if (scalar(@$uids = @{$over->uid_range($beg, $end, $sql)})) { $range_info->[0] = $uids->[-1] + 1; # update $beg @@ -1114,7 +1115,7 @@ sub parse_imap_query ($$) { my ($self, $query) = @_; my $q = PublicInbox::IMAPsearchqp::parse($self, $query); if (ref($q)) { - my $max = $self->{ibx}->over->max; + my $max = $self->{ibx}->over(1)->max; my $beg = 1; uid_clamp($self, \$beg, \$max); $q->{range_info} = [ $beg, $max ]; diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index b94ffdb0..df140cac 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -8,6 +8,7 @@ use PublicInbox::Git; use PublicInbox::MID qw(mid2path); use PublicInbox::Eml; use List::Util qw(max); +use Carp qw(croak); # Long-running "git-cat-file --batch" processes won't notice # unlinked packs, so we need to restart those processes occasionally. @@ -168,8 +169,8 @@ sub max_git_epoch { } sub mm { - my ($self) = @_; - $self->{mm} ||= eval { + my ($self, $req) = @_; + $self->{mm} //= eval { require PublicInbox::Msgmap; my $dir = $self->{inboxdir}; if ($self->version >= 2) { @@ -177,7 +178,7 @@ sub mm { } else { PublicInbox::Msgmap->new($dir); } - }; + } // ($req ? croak("E: $@") : undef); } sub search { @@ -195,19 +196,19 @@ sub search { sub isrch { $_[0]->{isrch} // search($_[0]) } sub over { - $_[0]->{over} //= eval { - my $srch = $_[0]->{search} //= do { - _cleanup_later($_[0]); + my ($self, $req) = @_; + $self->{over} //= eval { + my $srch = $self->{search} //= do { + _cleanup_later($self); require PublicInbox::Search; - PublicInbox::Search->new($_[0]); + PublicInbox::Search->new($self); }; my $over = PublicInbox::Over->new("$srch->{xpfx}/over.sqlite3"); $over->dbh; # may fail $over; - }; + } // ($req ? croak("E: $@") : undef); } - sub try_cat { my ($path) = @_; open(my $fh, '<', $path) or return ''; diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 9df47133..13a68bb8 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -202,7 +202,7 @@ sub cmd_list ($;$$) { sub listgroup_range_i { my ($self, $beg, $end) = @_; - my $r = $self->{ibx}->mm->msg_range($beg, $end, 'num'); + my $r = $self->{ibx}->mm(1)->msg_range($beg, $end, 'num'); scalar(@$r) or return; $self->msg_more(join('', map { "$_->[0]\r\n" } @$r)); 1; @@ -210,7 +210,7 @@ sub listgroup_range_i { sub listgroup_all_i { my ($self, $num) = @_; - my $ary = $self->{ibx}->mm->ids_after($num); + my $ary = $self->{ibx}->mm(1)->ids_after($num); scalar(@$ary) or return; more($self, join("\r\n", @$ary)); 1; @@ -261,7 +261,7 @@ sub parse_time ($$;$) { sub group_line ($$) { my ($self, $ibx) = @_; - my ($min, $max) = $ibx->mm->minmax; + my ($min, $max) = $ibx->mm(1)->minmax; more($self, "$ibx->{newsgroup} $max $min n"); } @@ -367,7 +367,7 @@ sub cmd_group ($$) { $nntpd->idler_start; $self->{ibx} = $ibx; - my ($min, $max) = $ibx->mm->minmax; + my ($min, $max) = $ibx->mm(1)->minmax; $self->{article} = $min; my $est_size = $max - $min; "211 $est_size $min $max $group"; @@ -381,7 +381,7 @@ sub article_adj ($$) { defined $n or return '420 no current article has been selected'; $n += $off; - my $mid = $ibx->mm->mid_for($n); + my $mid = $ibx->mm(1)->mid_for($n); unless ($mid) { $n = $off > 0 ? 'next' : 'previous'; return "421 no $n article in this group"; @@ -418,9 +418,8 @@ sub xref_by_tc ($$$) { $by_addr->{lc($_)} // () } (PublicInbox::Address::emails($smsg->{$f} // '')); for my $ibx (@ibxs) { - my $ngname = $ibx->{newsgroup} // next; - next if defined $xref->{$ngname}; - $xref->{$ngname} = eval { $ibx->mm->num_for($mid) }; + $xref->{$ibx->{newsgroup}} //= + $ibx->mm(1)->num_for($mid); } } } @@ -437,13 +436,15 @@ sub xref ($$$) { $xref = { $cur_ng => $smsg->{num} }; my $mid = $smsg->{mid}; for my $ibx (values %{$nntpd->{pi_cfg}->{-by_newsgroup}}) { - next if defined($xref->{$ibx->{newsgroup}}); - my $num = eval { $ibx->mm->num_for($mid) } // next; - $xref->{$ibx->{newsgroup}} = $num; + $xref->{$ibx->{newsgroup}} //= + $ibx->mm(1)->num_for($mid); } } my $ret = "$nntpd->{servername} $cur_ng:".delete($xref->{$cur_ng}); - $ret .= " $_:$xref->{$_}" for (sort keys %$xref); + for my $ng (sort keys %$xref) { + my $num = $xref->{$ng} // next; + $ret .= " $ng:$num"; + } $ret; } @@ -504,7 +505,7 @@ find_ibx: return '412 no newsgroup has been selected'; } found: - my $smsg = $ibx->over->get_art($n) or return $err; + my $smsg = $ibx->over(1)->get_art($n) or return $err; $smsg->{-ibx} = $ibx; if ($code == 223) { # STAT set_art($self, $n); @@ -618,7 +619,7 @@ sub get_range ($$) { my $ibx = $self->{ibx} or return '412 no news group has been selected'; defined $range or return '420 No article(s) selected'; my ($beg, $end); - my ($min, $max) = $ibx->mm->minmax; + my ($min, $max) = $ibx->mm(1)->minmax; if ($range =~ /\A([0-9]+)\z/) { $beg = $end = $1; } elsif ($range =~ /\A([0-9]+)-\z/) { @@ -688,7 +689,7 @@ sub long_response ($$;@) { sub hdr_msgid_range_i { my ($self, $beg, $end) = @_; - my $r = $self->{ibx}->mm->msg_range($beg, $end); + my $r = $self->{ibx}->mm(1)->msg_range($beg, $end); @$r or return; $self->msg_more(join('', map { "$_->[0] <$_->[1]>\r\n" } @$r)); 1; @@ -714,7 +715,7 @@ sub mid_lookup ($$) { my ($self, $mid) = @_; my $cur_ibx = $self->{ibx}; if ($cur_ibx) { - my $n = $cur_ibx->mm->num_for($mid); + my $n = $cur_ibx->mm(1)->num_for($mid); return ($cur_ibx, $n) if defined $n; } my $pi_cfg = $self->{nntpd}->{pi_cfg}; @@ -744,7 +745,7 @@ EOF } else { # slow path for non-ALL users for my $ibx (values %{$pi_cfg->{-by_newsgroup}}) { next if defined $cur_ibx && $ibx eq $cur_ibx; - my $n = $ibx->mm->num_for($mid); + my $n = $ibx->mm(1)->num_for($mid); return ($ibx, $n) if defined $n; } } @@ -754,7 +755,7 @@ EOF sub xref_range_i { my ($self, $beg, $end) = @_; my $ibx = $self->{ibx}; - my $msgs = $ibx->over->query_xover($$beg, $end); + my $msgs = $ibx->over(1)->query_xover($$beg, $end); scalar(@$msgs) or return; $$beg = $msgs->[-1]->{num} + 1; $self->msg_more(join('', map { @@ -770,7 +771,7 @@ sub hdr_xref ($$$) { # optimize XHDR Xref [range] for rtin my $mid = $1; my ($ibx, $n) = mid_lookup($self, $mid); return r430 unless $n; - my $smsg = $ibx->over->get_art($n) or return; + my $smsg = $ibx->over(1)->get_art($n) or return; hdr_mid_response($self, $xhdr, $ibx, $n, $range, xref($self, $ibx, $smsg)); } else { # numeric range @@ -783,16 +784,15 @@ sub hdr_xref ($$$) { # optimize XHDR Xref [range] for rtin } sub over_header_for { - my ($over, $num, $field) = @_; - my $smsg = $over->get_art($num) or return; + my ($ibx, $num, $field) = @_; + my $smsg = $ibx->over(1)->get_art($num) or return; return PublicInbox::Smsg::date($smsg) if $field eq 'date'; $smsg->{$field}; } sub smsg_range_i { my ($self, $beg, $end, $field) = @_; - my $over = $self->{ibx}->over; - my $msgs = $over->query_xover($$beg, $end); + my $msgs = $self->{ibx}->over(1)->query_xover($$beg, $end); scalar(@$msgs) or return; my $tmp = ''; @@ -816,7 +816,7 @@ sub hdr_smsg ($$$$) { if (defined $range && $range =~ $ONE_MSGID) { my ($ibx, $n) = mid_lookup($self, $1); return r430 unless defined $n; - my $v = over_header_for($ibx->over, $n, $field); + my $v = over_header_for($ibx, $n, $field); hdr_mid_response($self, $xhdr, $ibx, $n, $range, $v); } else { # numeric range $range = $self->{article} unless defined $range; @@ -885,7 +885,7 @@ sub hdr_mid_response ($$$$$$) { sub xrover_i { my ($self, $beg, $end) = @_; - my $h = over_header_for($self->{ibx}->over, $$beg, 'references'); + my $h = over_header_for($self->{ibx}, $$beg, 'references'); more($self, "$$beg $h") if defined($h); $$beg++ < $end; } @@ -925,7 +925,7 @@ sub cmd_over ($;$) { if ($range && $range =~ $ONE_MSGID) { my ($ibx, $n) = mid_lookup($self, $1); defined $n or return r430; - my $smsg = $ibx->over->get_art($n) or return r430; + my $smsg = $ibx->over(1)->get_art($n) or return r430; more($self, '224 Overview information follows (multi-line)'); # Only set article number column if it's the current group @@ -946,7 +946,7 @@ sub cmd_over ($;$) { sub xover_i { my ($self, $beg, $end) = @_; my $ibx = $self->{ibx}; - my $msgs = $ibx->over->query_xover($$beg, $end); + my $msgs = $ibx->over(1)->query_xover($$beg, $end); my $nr = scalar @$msgs or return; # OVERVIEW.FMT @@ -1014,7 +1014,7 @@ sub cmd_xpath ($$) { } } else { # slow path, no point in using long_response for my $ibx (values %$groups) { - my $n = $ibx->mm->num_for($mid) // next; + my $n = $ibx->mm(1)->num_for($mid) // next; push @paths, "$ibx->{newsgroup}/$n"; } }