1124569 - When CONDSTORE is used and IDLE is not used, if a mail is deleted/expunged by other client, * n EXPUNGE response to noop upon next new maail check by Biff is ignored, and msgDBHdr of the mail is not removed.

Status: ASSIGNED

(MailNews Core :: Networking: IMAP, defect)

Description

[WADA:World Anti-bad-Duping Agency]

+++ This bug was initially created as a clone of Bug #1123094 +++
This is spin-off of bug 1123094 comment #10.

When CONDSTORE is used and IDLE is not used, if a mail is deleted/expunged by other client,
* n EXPUNGE response to noop upon next new maail check by Biff  is ignored, and msgDBHdr of the mail is not removed.
If CONDSTORE support is disabled, msgDBHdr of the deleted UID was normlly removed after * 1 EXPUNGE .
It looks CONDSTORE only problem.

0. Two Gmail IMAP accounts with same userName :
            imap#1 : imap.googlemail.com,  imaap#2 : imaap.gmail,com
      CONDSTORE support is enbled.
      imap.gmail.com definition, IDLE command use  Disabled, IMAP delete model = Move to Trash
      Max cached connections = 3, so Inbox is always selected at a cached connection.
      6 mails are held. UID=819, 820, 821, 822, 823,  825

1.  at imap.googlemil.com
     delete UID~819, EXPUNGE, change Tag of UID=825(MODSEQ =187430)

2. at imap.gmail.com, next new mail fetch by Biff is kicked.
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:SendData: 362 noop 
> [3944] 2612[10121d0]: 9886800:imap.gmail.com:S-[Gmail]/All Mail:SendData: 189 noop 
> [3944] 2612[10121d0]: ReadNextLine [stream=119993a0 nb=16 needmore=0]
> [3944] 2612[10121d0]: 9886800:imap.gmail.com:S-[Gmail]/All Mail:CreateNewLineFromSocket: 189 OK Success 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=13 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: * 1 EXPUNGE 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=12 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: * 5 EXISTS 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=16 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: 362 OK Success 
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:SendData: 363 getquotaroot "INBOX" 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=24 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: * QUOTAROOT "INBOX" "" 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=36 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: * QUOTA "" (STORAGE 6229 15728640) 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=16 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: 363 OK Success 
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:SendData: 364 UID fetch 826:* (FLAGS) 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=76 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: * 5 FETCH (UID 825 MODSEQ (187430) FLAGS ($label4 NonJunk tag4 tag_after)) 
> [3944] 3488[10119f0]: ReadNextLine [stream=11999340 nb=16 needmore=0]
> [3944] 3488[10119f0]: 922a000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: 364 OK Success 

3.Because \Delete flag of UID=819 is not notified to Tb, msgDBHdr of UID=819 is not removed.
   "* 1 EXPUNGE" ws returned to noop, so I thought some kind of action like "uid fetch 819 Flags" is done, but nothing was executedd for UID=819.
   (* 1 EXPUNGE == message #1 is EXPUNGEd, or 1 message was EXPUNGEd)
   And msgDBHdr for UID=819 was not removed.
   If UID=819 is clcked, blank message body was shown.
   Inbox is Offline-Use=Off folder. IIRC, I clicked UID=819 once aaand small text mail, so data was held in Disk or Memory Cache.
   It looks Disk/Memory Cache data is cleared by * 1 EXPUNGE.
   Because "uid fetch 1:* Flags"(without CHANGEDSINCE) won't be issued, there is no chance to remove UID=819.

5. Go Work Offline, Go Work Online, click Inbox
    => "uid fetch 1:* Flags"(without CHANGEDSINCE)  was issued, so msgDBHdr for UID=819 was removed.
    If first access is by new mail fetch of Bifff, "uid fetch 1:* Flags (CHANGEDSINCE 187430)" is issued,
    so non-removed msgDBHdr continuoisy remains.

Comment 1

[WADA:World Anti-bad-Duping Agency]

Why "* n EXPuNGE" is not processed is herhaps as follows.
  1. First sync after SELECT is partial and "uid fetch 1:* Flags (CHANGEDSINCE known_modseq)" is used, so, message number is not usable. 
  2. Because "* n EXPUNGE" response is based on messaage number,  "* n EXPUNGE" can not be processed.
This is prerhaps why QRESYNC is needed.
However, because "one mail has been expunged" is notified, Tb should do some actions to know !which maail has been expunged".
Because full resync(uid fetch 1:* flags  without CHANGEDSINCE) is used by Tb if "explicit folder open by folder click", even when  CONDSTORE is usable, "full resync due to detection of expunge" is surely possible.
I believe CONDSTORE never prohibits "full resync" by "uid fetch 1:* Flags without CONDSTORE".

A possible simplest solution before QRESYNC will be supported by Tb is:
   Always do "full resync" == "uid fetch 1:* Flags without CONDSTORE" after SELECT, even when CONDSTORE is usable.
However, to gain fruit by CONDSTORE, following may be better:
      (a) do partial resync always ==uid fetch 1:* Flags (CHANEDSINCE known_modseq) at both "after SELECT" and "upon each new mail check"
  + (b) full resync if expunge is detected  == uid fetch 1:* Flags witout CHANGEDSiNCE as done when folder is explicitly opened.

Comment 2

[gene smith]

I have done some experiments with CONDSTORE enabled with gmail and see the same results that WADA reported here. If I have 2 independent TB clients both accessing the same gmail account and I delete a message on one (will call it remote), on the other sometime the message goes away and sometimes it remains. If it remains, when click it, I see an empty reading pane.

I have found that if only a "partial" sync occurs, then the message stays on the list when deleted by the remote client. But if a full sync occurs, the message goes away when deleted with the remote client (full fetch called "sanity check"[*] in the code).

The partial sync occurs when tb is restarted and the mailbox has not changed since the previous start-up. If a message is removed or added with the remote client, when tb starts up, a full sync occurs.

When only a partial sync occurs, there are some "arrays" in nsImapFlagAndUidState.cpp that are not completely populated since the imap FETCH response is only partial. These arrays are called fUids and fFlags [**]. The partial fetch response only populates the array element at the highest UID for each array. A full fetch response (like always occurs when CONDSTORE is not enabled or when sanity check occurs) populates all the array elements -- one element for each email in the mailbox.

When the email is deleted on the remote client, and the other client has only done a partial fetch/sync, when the "* msgNo EXPUNGE" imap response occurs (at idle response or biff), the code attempts to remove the element of fUids and fFlags at msgNo. But since this index is not populated, there is no effect and the message is not removed from the list.

What I don't understand (yet) is how fUids and fFlags arrays tie into whether an email is shown or not shown on the list, i.e., per bug summary, msgDBHdr of the mail is removed or not removed. If someone could explain this I would appreciate it.

One thing I have discovered is that when CONDSTORE was "temporarily" disabled by default back in bug 912216[***], that it was observed that gmail flag changes were not reported by IDLE. I don't see a problem with that now. I can set an email read/unread or starred/unstarred on the remote client and the changes appear correctly and automatically on the other client via the IDLE response.

[*] https://dxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapProtocol.cpp#4173
[**]https://dxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapFlagAndUidState.cpp#129
[***]This disables CONDSTORE for all imap servers, not just gmail.

Comment 3

[gene smith]

(In reply to gene smith from comment #2)
> 
> What I don't understand (yet) is how fUids and fFlags arrays tie into
> whether an email is shown or not shown on the list, i.e., per bug summary,
> msgDBHdr of the mail is removed or not removed. If someone could explain
> this I would appreciate it.
> 

I understand this now and I have a proposed fix for this bug. The main problem is that the imap expunge response only provides the message sequence number and not the UID. Since the UID is the "key" into the database, it must be known in order to remove the entry for the expunged message and the code fails to make the correlation when CONDSTORE is enabled and "partial" flag fetches cause unpopulated fUids[msg seq num] array elements containing a not-yet-initialized and invalid zero UID value. 

The CONDSTORE extension QRESYNC replaces the base expunge response with a "vanished" response that includes the UID. This would greatly help with this bug. However, QRESYNC is not currently supported in tb and it is not supported nor is it planned to be supported in gmail, although tb and gmail supports base CONDSTORE. (See http://mailman13.u.washington.edu/pipermail/imap-protocol/2018-September/002630.html)

My fix adds code to correlate the expunged message sequence number from the standard "expunge" imap response to the proper UID with CONDSTORE and partial fetches in use, so the message can be properly removed from the database. A patch is forthcoming. 

Note1: This bug is also somewhat related to and discussed in Bug 1428097 Comment 28.

Note2: Typically, servers that support CONDSTORE also support QRESYNC, e.g., Dovecot. I think gmail is the only well known imap server that supports CONDSTORE without supporting QRESYNC. My proposed fix may make support for QRESYNC in tb unnecessary since, in a way, it simulates what QRESYNC does, at least in regard to the "vanished UID" response feature.

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to gene smith from comment #3)

...
Note2: Typically, servers that support CONDSTORE also support QRESYNC, e.g.,
Dovecot. I think gmail is the only well known imap server that supports
CONDSTORE without supporting QRESYNC. My proposed fix may make support for
QRESYNC in tb unnecessary since, in a way, it simulates what QRESYNC does,
at least in regard to the "vanished UID" response feature.

Is this then the repair for Bug 912216 - Temporarily disable CONDSTORE support for many "GMail IMAP ?

Comment 5

[gene smith]

(In reply to Wayne Mery (:wsmwk) from comment #4)

(In reply to gene smith from comment #3)

...
Note2: Typically, servers that support CONDSTORE also support QRESYNC, e.g.,
Dovecot. I think gmail is the only well known imap server that supports
CONDSTORE without supporting QRESYNC. My proposed fix may make support for
QRESYNC in tb unnecessary since, in a way, it simulates what QRESYNC does,
at least in regard to the "vanished UID" response feature.

Is this then the repair for Bug 912216 - Temporarily disable CONDSTORE support for many "GMail IMAP ?

Yes, I was hoping for this to allow CONDSTORE to be turned back on by default, or at least be less broken. However, I haven't had a chance to get back to this in over a year. (Right now I'm working the "critical" bug for doing bulk copies/moves between servers, Bug 538375, but not easy.)