[BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[Eliza Velasquez]

Hello notmuch,

As the subject says, it seems like the filename cache for a message is
not reliably invalidated when notmuch_message_tags_to_maildir_flags is
called, causing the following series of function calls to behave
unexpectedly.

- notmuch_database_index_file
- notmuch_message_get_filenames
- notmuch_message_tags_to_maildir_flags
- notmuch_message_get_filenames (stale list returned)

According to the investigator [1], this happens about 10% of the time,
but it's possible that this is just because there is some quirk in the
client-side code causing it to trigger only sometimes.

[1] https://github.com/elizagamedev/mujmap/pull/10#issuecomment-1131587169

-- 
Eliza

Re: [BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[David Bremner]

Eliza Velasquez <eliza@eliza.sh> writes:

> Hello notmuch,
>
> As the subject says, it seems like the filename cache for a message is
> not reliably invalidated when notmuch_message_tags_to_maildir_flags is
> called, causing the following series of function calls to behave
> unexpectedly.
>
> - notmuch_database_index_file
> - notmuch_message_get_filenames
> - notmuch_message_tags_to_maildir_flags
> - notmuch_message_get_filenames (stale list returned)
>
> According to the investigator [1], this happens about 10% of the time,
> but it's possible that this is just because there is some quirk in the
> client-side code causing it to trigger only sometimes.

that cache is supposed to be invalidated by calls to
_notmuch_message_add_filename and
_notmuch_message_remove_filename. There is one unchecked call to
_notmuch_message_add_term that looks a bit suspect. The obvious thing
that can go wrong is filename getting too long. Here is a patch to maybe
return better diagnostics in that case

diff --git a/lib/message.cc b/lib/message.cc
index 63b216b6..bebc5f6a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -169,6 +169,7 @@ _notmuch_message_create_for_document (const void *talloc_owner,
 
     message->doc = doc;
     message->termpos = 0;
+    message->modified = FALSE;
 
     return message;
 }
@@ -937,6 +938,7 @@ _notmuch_message_add_filename (notmuch_message_t *message,
 {
     const char *relative, *directory;
     notmuch_status_t status;
+    notmuch_private_status_t private_status;
     void *local = talloc_new (message);
     char *direntry;
 
@@ -960,7 +962,11 @@ _notmuch_message_add_filename (notmuch_message_t *message,
 
     /* New file-direntry allows navigating to this message with
      * notmuch_directory_get_child_files() . */
-    _notmuch_message_add_term (message, "file-direntry", direntry);
+    private_status = _notmuch_message_add_term (message, "file-direntry", direntry);
+    status = COERCE_STATUS (private_status,
+                           "Unexpected error from _notmuch_message_add_term");
+    if (status)
+       return status;
 
     _notmuch_message_add_folder_terms (message, directory);
     _notmuch_message_add_path_terms (message, directory);

The other issue that might cause intermittent failures is uncaught
exceptions. We cleaned a bunch of those up a few years ago, but it looks
like we missed a few spots. Off hand I'm not sure what would be causing
such exceptions, but _notmuch_message_add_term and
_notmuch_message_remove_term should probably have the Xapian operations
wrapped in try / catch blocks.

It might be worth running the failing test under gdb to see if you can
spot exceptions.

Re: [BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[David Bremner]

Eliza Velasquez <eliza@eliza.sh> writes:

> Hello notmuch,
>
> As the subject says, it seems like the filename cache for a message is
> not reliably invalidated when notmuch_message_tags_to_maildir_flags is
> called, causing the following series of function calls to behave
> unexpectedly.
>
> - notmuch_database_index_file
> - notmuch_message_get_filenames
> - notmuch_message_tags_to_maildir_flags
> - notmuch_message_get_filenames (stale list returned)
>
> According to the investigator [1], this happens about 10% of the time,
> but it's possible that this is just because there is some quirk in the
> client-side code causing it to trigger only sometimes.
>
> [1] https://github.com/elizagamedev/mujmap/pull/10#issuecomment-1131587169
>

I have been looking at this code, and making some small improvements [1],
but without a C reproducer I don't really know if it helps the
submitter's issue.

d

[1]: https://nmbug.notmuchmail.org/nmweb/show/20220523233901.3506880-1-david@tethera.net

Re: [BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[Eliza Velasquez]

On Tue, May 24 2022 at 09:45 -03, David Bremner <david@tethera.net> wrote:

> Eliza Velasquez <eliza@eliza.sh> writes:
>
>> Hello notmuch,
>>
>> As the subject says, it seems like the filename cache for a message is
>> not reliably invalidated when notmuch_message_tags_to_maildir_flags is
>> called, causing the following series of function calls to behave
>> unexpectedly.
>>
>> - notmuch_database_index_file
>> - notmuch_message_get_filenames
>> - notmuch_message_tags_to_maildir_flags
>> - notmuch_message_get_filenames (stale list returned)
>>
>> According to the investigator [1], this happens about 10% of the time,
>> but it's possible that this is just because there is some quirk in the
>> client-side code causing it to trigger only sometimes.
>>
>> [1] https://github.com/elizagamedev/mujmap/pull/10#issuecomment-1131587169
>>
>
> I have been looking at this code, and making some small improvements [1],
> but without a C reproducer I don't really know if it helps the
> submitter's issue.
>
> d
>
> [1]: https://nmbug.notmuchmail.org/nmweb/show/20220523233901.3506880-1-david@tethera.net

Thank you for looking into this and putting out such a quick patch, I
really appreciate it. It turns out the core issue that we were
encountering actually seems like it's due to cache propagation issues
across message instances, according to the investigator (Rob). To quote:

> I think its a fairly standard cache write propagation issue, with the
> outstanding message objects as the caches. Whether or not its a bug in
> notmuch itself I think is entirely down to whether holding and
> modifying multiple message objects for the "same" record (with
> duplicates) is supposed to work or not. I'm in two minds about it; on
> the one hand, multiple message objects is clearly a valid use case
> (queries!), on the other, playing with the mail files themselves is a
> bit of a niche feature. I'll think about it a bit more and maybe take
> it up to the notmuch list myself.

He wrote a more detailed writeup of it as well [1], which includes a
minimal reproducible test case written in C. After reviewing this, I'm
also not sure whether or not this should be considered a notmuch bug,
but it was surprising behavior.

[1] https://gist.github.com/robn/a56420062127238f57541eb47bc962f5

Re: [BUG] notmuch_message_tags_to_maildir_flags does not always invalidate filenames_list

[David Bremner]

Eliza Velasquez <eliza@eliza.sh> writes:

> He wrote a more detailed writeup of it as well [1], which includes a
> minimal reproducible test case written in C. After reviewing this, I'm
> also not sure whether or not this should be considered a notmuch bug,
> but it was surprising behavior.
>
> [1] https://gist.github.com/robn/a56420062127238f57541eb47bc962f5

At this point I'll wait for further discussion on the list. I suspect
the issue will be not be too easy to fix, but we can probably improve
the documentation.

d