Skip to content

Commit

Permalink
imap: resolve unstable EXPUNGE observability
Browse files Browse the repository at this point in the history
Following the issuance of the EXPUNGE IMAP command by a client, and
after imapd has deleted e.g. n=7 mails at once via the midb M-DELE
command, imapd would erratically respond with fewer than n EXPUNGE
responses, deferring the remaining EXPUNGE responses until the next
opportunity.

Even though EXPUNGE events were sent to other imapd threads, the
current thread was skipped, so it had to rely on loopback events —
which do not arrive timely.
  • Loading branch information
jengelh committed Oct 6, 2024
1 parent ebbff8d commit 0b2b5ae
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
1 change: 1 addition & 0 deletions mra/imap/imap_cmd_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,7 @@ int imap_cmd_parser_append(int argc, char **argv, imap_context *pcontext) try
auto imap_reply_str1 = resource_get_imap_code(1715, 2);
std::string buf;
for (i=0; i<10; i++) {
// wait for midb's actions showing up... woah terrible
uint32_t uidvalid = 0;
if (system_services_summary_folder(pcontext->maildir,
temp_name, nullptr, nullptr, nullptr, &uidvalid, nullptr,
Expand Down
37 changes: 26 additions & 11 deletions mra/imap/imap_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,28 @@ static std::vector<imap_context *> *sh_query(const char *x)
return i == g_select_hash.end() ? nullptr : &i->second;
}

/*
* The event bus does not loopback messages to the same connection id (PID),
* so, generally, anything that needs to be conveyed to imapd sibling threads
* we have to do via shared memory. Fair enough.
*
* But there is a caveat: exmdb actions loop do back as notifications to midb,
* which in turn goes back imapd, like so:
*
* - imapd issues M-DELE command to midb
* - midb issues delete_message EXRPC to exmdb
* - exmdb notifies midb due to a mailbox-wide subscription midb asked for
* - midb issues MESSAGE-EXPUNGED on the event bus
* - imapd receives MESSAGE-EXPUNGED event
*
* imap:APPEND commands actually behaves similar, leading to a
* FOLDER-TOUCH event coming back.
*
* Notifications arrive asynchronously though, and thus with a fair chance to
* be "late", such that the EXPUNGE command would respond with fewer EXPUNGE
* responses than the number of messages to delete.
*/

void imap_parser_bcast_touch(const imap_context *current, const char *username,
const char *folder)
{
Expand Down Expand Up @@ -1196,28 +1218,21 @@ void imap_parser_bcast_expunge(const imap_context &current,
char user_lo[UADDR_SIZE];
gx_strlcpy(user_lo, current.username, std::size(user_lo));
HX_strlower(user_lo);
/*
* gromox-event does not send back events to our own process.
* (So, we have to edit the data structures for our other threads
* from this function.)
* Benefit: no IPC roundtrip to notify other threads.
* Downside: Some code duplication between imap_parser_event_expunge
* and imap_parser_bcast_expunge.
*/

/* Distribute expunges via shared memory as mentioned earlier. */
std::unique_lock hl_hold(g_hash_lock);
auto ctx_list = sh_query(user_lo);
if (ctx_list == nullptr)
return;
for (auto &other : *ctx_list) {
if (&current == other ||
strcmp(current.selected_folder, other->selected_folder) != 0)
if (strcmp(current.selected_folder, other->selected_folder) != 0)
continue;
for (auto p : exp_list)
other->f_expunged_uids.emplace_back(p->uid);
other->async_change_mask |= REPORT_EXPUNGE;
}
hl_hold.unlock();
/* Bcast to other entities */
/* Bcast to other bus listeners (IOW, pop3) */
auto cmd = "MESSAGE-EXPUNGE "s + user_lo + " " + current.selected_folder + " ";
auto csize = cmd.size();
for (auto p : exp_list) {
Expand Down

0 comments on commit 0b2b5ae

Please sign in to comment.