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-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 900231F5B1 for ; Thu, 25 Jun 2020 10:09:37 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] imap: EXAMINE: avoid potential race conditions Date: Thu, 25 Jun 2020 10:09:37 +0000 Message-Id: <20200625100937.5552-3-e@yhbt.net> In-Reply-To: <20200625100937.5552-1-e@yhbt.net> References: <20200625100937.5552-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: We need to rely on num_highwater for UIDNEXT since the highest `num' stored in over.sqlite3 may be rolled back if the most recent messages were spam. We also need to load the uo2m immediately on EXAMINE to ensure EXISTS responses are always consistent with regard to future updates. --- lib/PublicInbox/IMAP.pm | 61 ++++++++++++++++++++++------------------- lib/PublicInbox/Over.pm | 18 ++---------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm index 888c9becfe0..9ad74de837f 100644 --- a/lib/PublicInbox/IMAP.pm +++ b/lib/PublicInbox/IMAP.pm @@ -192,8 +192,8 @@ sub cmd_capability ($$) { # uo2m: UID Offset to MSN, this is an arrayref by default, # but uo2m_hibernate can compact and deduplicate it -sub uo2m_ary_new ($) { - my ($self) = @_; +sub uo2m_ary_new ($;$) { + my ($self, $exists) = @_; my $base = $self->{uid_base}; my $uids = $self->{ibx}->over->uid_range($base + 1, $base + UID_SLICE); @@ -202,6 +202,7 @@ sub uo2m_ary_new ($) { my $msn = 0; ++$base; $tmp[$_ - $base] = ++$msn for @$uids; + $$exists = $msn if $exists; \@tmp; } @@ -385,44 +386,48 @@ sub ensure_slices_exist ($$$) { push @$l, map { qq[* LIST (\\HasNoChildren) "." $_\r\n] } @created; } -sub inbox_lookup ($$) { - my ($self, $mailbox) = @_; - my ($ibx, $exists, $uidnext, $uid_base); - if ($mailbox =~ /\A(.+)\.([0-9]+)\z/) { - # old mail: inbox.comp.foo.$SLICE_IDX - my $mb_top = $1; - $uid_base = $2 * UID_SLICE; - $ibx = $self->{imapd}->{mailboxes}->{lc $mailbox} or return; - my $max; - ($exists, $uidnext, $max) = $ibx->over->imap_status($uid_base, - $uid_base + UID_SLICE); - ensure_slices_exist($self->{imapd}, $ibx, $max); - } else { # check for dummy inboxes - $mailbox = lc $mailbox; - $ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return; - +sub inbox_lookup ($$;$) { + my ($self, $mailbox, $examine) = @_; + my ($ibx, $exists, $uidmax, $uid_base) = (undef, 0, 0, 0); + $mailbox = lc $mailbox; + $ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return; + my $over = $ibx->over; + if ($over != $ibx) { # not a dummy + $mailbox =~ /\.([0-9]+)\z/ or + die "BUG: unexpected dummy mailbox: $mailbox\n"; + $uid_base = $1 * UID_SLICE; + + # ->num_highwater caches for writers, so use ->meta_accessor + $uidmax = $ibx->mm->meta_accessor('num_highwater') // 0; + if ($examine) { + $self->{uid_base} = $uid_base; + $self->{ibx} = $ibx; + $self->{uo2m} = uo2m_ary_new($self, \$exists); + } else { + $exists = $over->imap_exists; + } + ensure_slices_exist($self->{imapd}, $ibx, $over->max); + } else { + if ($examine) { + $self->{uid_base} = $uid_base; + $self->{ibx} = $ibx; + delete $self->{uo2m}; + } # 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); } - - $uid_base = $exists = 0; - $uidnext = 1; } - ($ibx, $exists, $uidnext, $uid_base); + ($ibx, $exists, $uidmax + 1, $uid_base); } sub cmd_examine ($$$) { my ($self, $tag, $mailbox) = @_; - my ($ibx, $exists, $uidnext, $base) = inbox_lookup($self, $mailbox); - return "$tag NO Mailbox doesn't exist: $mailbox\r\n" if !$ibx; - $self->{uid_base} = $base; - delete $self->{uo2m}; - # XXX: do we need this? RFC 5162/7162 my $ret = $self->{ibx} ? "* OK [CLOSED] previous closed\r\n" : ''; - $self->{ibx} = $ibx; + my ($ibx, $exists, $uidnext, $base) = inbox_lookup($self, $mailbox, 1); + return "$tag NO Mailbox doesn't exist: $mailbox\r\n" if !$ibx; $ret .= < 0 $sth->fetchrow_array // 0; } -sub imap_status { +sub imap_exists { my ($self, $uid_base, $uid_end) = @_; - my $dbh = $self->connect; - my $sth = $dbh->prepare_cached(<<'', undef, 1); + my $sth = $self->connect->prepare_cached(<<'', undef, 1); SELECT COUNT(num) FROM over WHERE num > ? AND num <= ? $sth->execute($uid_base, $uid_end); - my $exists = $sth->fetchrow_array; - - $sth = $dbh->prepare_cached(<<'', undef, 1); -SELECT MAX(num) + 1 FROM over WHERE num <= ? - - $sth->execute($uid_end); - my $uidnext = $sth->fetchrow_array; - - $sth = $dbh->prepare_cached(<<'', undef, 1); -SELECT MAX(num) FROM over WHERE num > 0 - - ($exists, $uidnext, $sth->fetchrow_array // 0); + $sth->fetchrow_array; } sub check_inodes {