The retries are hardcoded to a small number, and error handling aborts
than propagating errors from notmuch_database_reopen. These are both
somewhat justified by the assumption that most things that can go
wrong in Xapian::Database::reopen are rare and fatal (like running out
of memory or disk corruption).
---
 lib/message.cc                 | 152 ++++++++++++++++++++++++-----------------
 test/T640-database-modified.sh |   1 -
 2 files changed, 88 insertions(+), 65 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 2fb67d85..15e2f528 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -49,6 +49,9 @@ struct visible _notmuch_message {
     /* Message document modified since last sync */
     notmuch_bool_t modified;
 
+    /* last view of database the struct is synced with */
+    unsigned long last_view;
+
     Xapian::Document doc;
     Xapian::termcount termpos;
 };
@@ -110,6 +113,9 @@ _notmuch_message_create_for_document (const void 
*talloc_owner,
     message->flags = 0;
     message->lazy_flags = 0;
 
+    /* the message is initially not synchronized with Xapian */
+    message->last_view = 0;
+
     /* Each of these will be lazily created as needed. */
     message->message_id = NULL;
     message->thread_id = NULL;
@@ -314,6 +320,8 @@ static void
 _notmuch_message_ensure_metadata (notmuch_message_t *message)
 {
     Xapian::TermIterator i, end;
+    notmuch_bool_t success = FALSE;
+
     const char *thread_prefix = _find_prefix ("thread"),
        *tag_prefix = _find_prefix ("tag"),
        *id_prefix = _find_prefix ("id"),
@@ -327,73 +335,89 @@ _notmuch_message_ensure_metadata (notmuch_message_t 
*message)
      * slightly more costly than looking up individual fields if only
      * one field of the message object is actually used, it's a huge
      * win as more fields are used. */
+    for (int count=0; count < 3 && !success; count++) {
+       try {
+           i = message->doc.termlist_begin ();
+           end = message->doc.termlist_end ();
+
+           /* Get thread */
+           if (!message->thread_id)
+               message->thread_id =
+                   _notmuch_message_get_term (message, i, end, thread_prefix);
+
+           /* Get tags */
+           assert (strcmp (thread_prefix, tag_prefix) < 0);
+           if (!message->tag_list) {
+               message->tag_list =
+                   _notmuch_database_get_terms_with_prefix (message, i, end,
+                                                            tag_prefix);
+               _notmuch_string_list_sort (message->tag_list);
+           }
 
-    i = message->doc.termlist_begin ();
-    end = message->doc.termlist_end ();
-
-    /* Get thread */
-    if (!message->thread_id)
-       message->thread_id =
-           _notmuch_message_get_term (message, i, end, thread_prefix);
-
-    /* Get tags */
-    assert (strcmp (thread_prefix, tag_prefix) < 0);
-    if (!message->tag_list) {
-       message->tag_list =
-           _notmuch_database_get_terms_with_prefix (message, i, end,
-                                                    tag_prefix);
-       _notmuch_string_list_sort (message->tag_list);
-    }
+           /* Get id */
+           assert (strcmp (tag_prefix, id_prefix) < 0);
+           if (!message->message_id)
+               message->message_id =
+                   _notmuch_message_get_term (message, i, end, id_prefix);
+
+           /* Get document type */
+           assert (strcmp (id_prefix, type_prefix) < 0);
+           if (! NOTMUCH_TEST_BIT (message->lazy_flags, 
NOTMUCH_MESSAGE_FLAG_GHOST)) {
+               i.skip_to (type_prefix);
+               /* "T" is the prefix "type" fields.  See
+                * BOOLEAN_PREFIX_INTERNAL. */
+               if (*i == "Tmail")
+                   NOTMUCH_CLEAR_BIT (&message->flags, 
NOTMUCH_MESSAGE_FLAG_GHOST);
+               else if (*i == "Tghost")
+                   NOTMUCH_SET_BIT (&message->flags, 
NOTMUCH_MESSAGE_FLAG_GHOST);
+               else
+                   INTERNAL_ERROR ("Message without type term");
+               NOTMUCH_SET_BIT (&message->lazy_flags, 
NOTMUCH_MESSAGE_FLAG_GHOST);
+           }
 
-    /* Get id */
-    assert (strcmp (tag_prefix, id_prefix) < 0);
-    if (!message->message_id)
-       message->message_id =
-           _notmuch_message_get_term (message, i, end, id_prefix);
-
-    /* Get document type */
-    assert (strcmp (id_prefix, type_prefix) < 0);
-    if (! NOTMUCH_TEST_BIT (message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST)) {
-       i.skip_to (type_prefix);
-       /* "T" is the prefix "type" fields.  See
-        * BOOLEAN_PREFIX_INTERNAL. */
-       if (*i == "Tmail")
-           NOTMUCH_CLEAR_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
-       else if (*i == "Tghost")
-           NOTMUCH_SET_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
-       else
-           INTERNAL_ERROR ("Message without type term");
-       NOTMUCH_SET_BIT (&message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST);
+           /* Get filename list.  Here we get only the terms.  We lazily
+            * expand them to full file names when needed in
+            * _notmuch_message_ensure_filename_list. */
+           assert (strcmp (type_prefix, filename_prefix) < 0);
+           if (!message->filename_term_list && !message->filename_list)
+               message->filename_term_list =
+                   _notmuch_database_get_terms_with_prefix (message, i, end,
+                                                            filename_prefix);
+
+
+           /* Get property terms. Mimic the setup with filenames above */
+           assert (strcmp (filename_prefix, property_prefix) < 0);
+           if (!message->property_map && !message->property_term_list)
+               message->property_term_list =
+                   _notmuch_database_get_terms_with_prefix (message, i, end,
+                                                        property_prefix);
+
+           /* Get reply to */
+           assert (strcmp (property_prefix, replyto_prefix) < 0);
+           if (!message->in_reply_to)
+               message->in_reply_to =
+                   _notmuch_message_get_term (message, i, end, replyto_prefix);
+
+
+           /* It's perfectly valid for a message to have no In-Reply-To
+            * header. For these cases, we return an empty string. */
+           if (!message->in_reply_to)
+               message->in_reply_to = talloc_strdup (message, "");
+
+           /* all the way without an exception */
+           success = TRUE;
+       } catch (const Xapian::DatabaseModifiedError &error) {
+           notmuch_status_t status = notmuch_database_reopen 
(message->notmuch);
+           if (status != NOTMUCH_STATUS_SUCCESS)
+               INTERNAL_ERROR ("unhandled error from notmuch_database_reopen: 
%s\n",
+                               notmuch_status_to_string (status));
+           success = FALSE;
+       } catch (const Xapian::Error &error) {
+           INTERNAL_ERROR ("A Xapian exception occurred fetching message 
metadata: %s\n",
+                           error.get_msg().c_str());
+       }
     }
-
-    /* Get filename list.  Here we get only the terms.  We lazily
-     * expand them to full file names when needed in
-     * _notmuch_message_ensure_filename_list. */
-    assert (strcmp (type_prefix, filename_prefix) < 0);
-    if (!message->filename_term_list && !message->filename_list)
-       message->filename_term_list =
-           _notmuch_database_get_terms_with_prefix (message, i, end,
-                                                    filename_prefix);
-
-
-    /* Get property terms. Mimic the setup with filenames above */
-    assert (strcmp (filename_prefix, property_prefix) < 0);
-    if (!message->property_map && !message->property_term_list)
-       message->property_term_list =
-           _notmuch_database_get_terms_with_prefix (message, i, end,
-                                                    property_prefix);
-
-    /* Get reply to */
-    assert (strcmp (property_prefix, replyto_prefix) < 0);
-    if (!message->in_reply_to)
-       message->in_reply_to =
-           _notmuch_message_get_term (message, i, end, replyto_prefix);
-
-
-    /* It's perfectly valid for a message to have no In-Reply-To
-     * header. For these cases, we return an empty string. */
-    if (!message->in_reply_to)
-       message->in_reply_to = talloc_strdup (message, "");
+    message->last_view = message->notmuch->view;
 }
 
 void
diff --git a/test/T640-database-modified.sh b/test/T640-database-modified.sh
index 41869eaa..9599417c 100755
--- a/test/T640-database-modified.sh
+++ b/test/T640-database-modified.sh
@@ -5,7 +5,6 @@ test_description="DatabaseModifiedError handling"
 add_email_corpus
 
 test_begin_subtest "catching DatabaseModifiedError in 
_notmuch_message_ensure_metadata"
-test_subtest_known_broken
 test_C ${MAIL_DIR} <<'EOF'
 #include <unistd.h>
 #include <stdlib.h>
-- 
2.11.0

_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Reply via email to