user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 0/2] imap: fix some missing EXISTS responses
@ 2020-06-25 10:09  7% Eric Wong
  2020-06-25 10:09  6% ` [PATCH 2/2] imap: EXAMINE: avoid potential race conditions Eric Wong
  0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2020-06-25 10:09 UTC (permalink / raw)
  To: meta

I noticed there were still some cases mutt didn't pick up
messages that showed up and I think I've fixed the last of
them.

Been testing these throughout the day on public-inbox.org
and things seem good.

Eric Wong (2):
  imap: always send EXISTS on uo2m_extend
  imap: EXAMINE: avoid potential race conditions

 lib/PublicInbox/IMAP.pm | 90 ++++++++++++++++++++++-------------------
 lib/PublicInbox/Over.pm | 18 ++-------
 2 files changed, 52 insertions(+), 56 deletions(-)

^ permalink raw reply	[relevance 7%]

* [PATCH 2/2] imap: EXAMINE: avoid potential race conditions
  2020-06-25 10:09  7% [PATCH 0/2] imap: fix some missing EXISTS responses Eric Wong
@ 2020-06-25 10:09  6% ` Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2020-06-25 10:09 UTC (permalink / raw)
  To: meta

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 .= <<EOF;
 * $exists EXISTS\r
 * $exists RECENT\r
diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index 74c8fb86270..e5a980d59ea 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -248,25 +248,13 @@ SELECT MAX(num) FROM over WHERE num > 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 {

^ permalink raw reply related	[relevance 6%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-06-25 10:09  7% [PATCH 0/2] imap: fix some missing EXISTS responses Eric Wong
2020-06-25 10:09  6% ` [PATCH 2/2] imap: EXAMINE: avoid potential race conditions Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).