IMAP: Emails are not saved and displayed in the correct folder

[jermy-c]

Reproduction

1. Login into an IMAP account
2. Click Inbox
3. Click Starred or any other folder
4. Click Inbox again

Actual Result

1. Emails from other folders are displayed
2. Emails from other folders are saved also when checking the DB
3. Sometimes you may see the emails in the folder change in realtime, looks glitchy

Expected Result

1. Only the emails that belong in the folder are displayed and saved
2. No change emails change in realtime when viewing, but might be added because some more maybe loaded

[jermy-c]

It seems like:

1. The IMAP connection doesn't have any folder selected
2. It looses the selected folder after switching folders a number of times

[benbucksch]

@jermy-c Can you please find reproduction steps to make this reproducible on any mail account, at 100% reproducible. We need steps so that everybody will see the bug, every time they follow the steps.

[jermy-c]

Reproduction 1

1. Login into Gmail
2. Open dev tools, to check IMAP fetch logs
3. Select Inbox
4. Switch between folders with no delay in between for 15+ times until you see IMAP fetch logs running for a while
5. Select any folder except for Inbox, should see emails from other folders
6. Click folder properties, and you should see the local messages is more than the total messages
7. After a while it will show the correct emails but the wrong emails were saved

Reproduction 2

1. Open Mustang
2. Do all steps in Reproduction 1 after step 1

[benbucksch]

@jermy-c Is this specific to Gmail accounts? Or does the same happen for other IMAP accounts?

[jermy-c]

It happens for other IMAP accounts also.

[benbucksch]

I can reproduce it.

[benbucksch]

When I wait a little, the right messages appear in the right folder.

This appears to me to be a display problem only. It seems to be that the app is so busy that it cannot refresh the message list.

Normally, the folder.messages array is in the frontend, and should be replaced immediately with the new folder's message list. The code looks fine to me. It is in MailApp.svelte:

  $: messages = searchMessages ?? $selectedFolder?.messages ?? new ArrayColl<EMail>();

Here, $selectedFolder should observe the folder changes, immediately replace messages with the folder.messages of the new folder, and that should then propagate down to <VerticalLayout {messages}>, to <VerticalMessageList> and <FastList items={sortedMessages}>.

So, that should still not happen. I don't know where the bottleneck is.

[benbucksch]

  await folder.listMessages();

is also in the same file MailApp.svelte, in function loadFolder();

[benbucksch]

A few possible causes that I can think of:

• The fetch is trashing (= excessively using) our frontend/backend connection so much that the frontend cannot do anything
• The fetch is trashing the browser event loop so much that the frontend cannot anything
• The await folder.listMessages is blocking while the folder is loading, and that somehow prevents the frontend to update. I cannot see how that would happen, though.
• The line above $: messages = $selectedFolder?.messages somehow is prevented from working, due to the load. That should not be the case, though.
• Some other reason

[benbucksch]

More possible reasons:

• Something somewhere overwrites the actual folder.messages collection, or its contents. You would likely find that best using a debugger 1).
• FastList's $: showItems = $items.getIndexRange()... doesn't return fast enough.

1) Debugger: F12, Source tab, Open source file, click on line number to add a break point. Then step through the lines using the debugger's line-by-line or "go into function" continue buttons.

[benbucksch]

I'm also get errors "IMAP UID FETCH: Invalid uidset" and "Cannot read properties of undefined (reading 'messageId')" (both at the same time for the same messages), which suggests that something actually overwrites the folder.messages contents. This is scary. But it would explain some of these bugs we saw.

[benbucksch]

@jermy-c If you want to reproduce by clearing all stored emails, but not reconfigure the accounts, you can:

1. install sqlite3 commandline app (e.g. via brew or similar)
2. sqlite3 mail.db 'delete from email; delete from folder;'

As bash script, create mustang-clear-db.sh:

#!/bin/bash
sqlite3 "/Users/<you>/Library/Application Support/Mustang/mail.db" 'delete from email; delete from folder;'     

then chmod 755 mustang-clear-db.sh

[benbucksch]

It's not the UI refresh, and not a bottleneck. It's a race, but not resource starvation.

Once 2 folders show the same content, these 2 folders will consistently keep showing the same content, even if I switch to other folders and back, and even if other folders are correctly separated. E.g. If FolderA and FolderB wrongly show the same content (of FolderA), and FolderC and FolderD do not, I can switch from FolderB to FolderD and back to FolderB, and FolderB will still show the contents of FolderA, whereas FolderD did not. If it was a lag, the switch from FolderD to FolderB would show the msgs of FolderD, not FolderA.

So, somewhere, it's overwriting a variable. :fearful: And it's a race condition.

[benbucksch]

I think I know what it is: We're using only 1 IMAP connection to the server, per account, for all folders. And IMAP can only have 1 folder selected at the same time. And, the command to select a mailbox and to fetch its content are 2 different IMAP commands. I think the latter is killing us here.

IMAPFolder.runCommand()
...
      await conn.mailboxOpen(this.path);
...
      return await imapFunc(conn);

If we try to get the messages of 2 folders at the same time, then the first folderA.listMessages() might do an IMAP SELECT to change the current folder to FolderA, and the second folderB.listMessage() would then do a IMAP SELECT to change the current folder to FolderB at the same time, and then the first folderA does IMAP FETCH, but it gets the messages for FolderB, because that's now the current folder.

I think that's the bug!

Now, that's what ImapFlow's getMailboxLock() is for, and IMAPFolder.runCommand(..., doLock = true) can do exactly that. And that would most likely fix the bug. But that also made the whole app very slow. Because while many messages are being fetched, no individual messages can be fetched for display, and I cannot mark anything as read or deleted etc., which made the whole app show to use while a big fetch is running.

So, possible fixes:

• Use doLock = true everywhere again, but that makes the app very unresponsive.
• Use our new Lock() somehow
• Use one of the above locks, but release the lock earlier, before the results are processed. Not sure whether that helps, because it would still wait for the fetch to complete.
• Use multiple IMAP connections.
  • One IMAP connection for listing messages (one per folder?)
  • One IMAP connection for interactive use, like fetching the currently displayed email or for sending changes,
  • One IMAP connection for IMAP IDLE, i.e. receiving push mail and change notifications from the server. (One per folder?)

[benbucksch]

I decided to

• re-enable the locking, using getMailboxLock() from ImapFlow, and to
• create 2 IMAP connections:
  1. a main connection for state changes (mark read, move, delete etc.), and for interactive display of msgs that are not yet downloaded
  2. a second fetch connection for bulk fetches of message lists and all emails in a folder

Implemented in git commit e1ba0b64

This change should hopefully fix this bug. @jermy-c Could you please test it carefully?

Downside is that it's still notably slower than before, because we can download only 1 folder at a time. Even interactive display of messages, which should happen on the first connection, is still slow and blocked. Maybe by the database?