[RFC/PATCH 2/2] lib: handle DatabaseModifiedError in _notmuch_message_create

Subject: [RFC/PATCH 2/2] lib: handle DatabaseModifiedError in _notmuch_message_create

Date: Thu, 26 Jun 2025 15:07:40 +0200

To: notmuch@notmuchmail.org

Cc:

From: Anton Khirnov


If an open database is modified sufficiently by other callers, the open
instance becomes invalid and operations on it throw
DatabaseModifiedError. Per Xapian documentation, the caller is then
supposed to reopen the database and restart the query. This exception is
currently not handled in _notmuch_message_create(), leading to the
default handler abort()ing the process.

Catch this exception in _notmuch_message_create() and return an error
instead of crashing. Since the entire query becomes invalid - including
results that have already been read by the caller - this situation
cannot be handled by libnotmuch transparently. A new public function -
notmuch_messages_status() - is added to allow the callers to check
whether the messages iterator was exhausted or terminated early due to
a runtime error. This also allows memory allocation failure to be
signalled to the caller.
---

Something similar also needs to be done for the threads iterator, I'll
do that next if this approach is acceptable.

---
 lib/message.cc                          |  4 +++
 lib/messages.c                          | 31 +++++++++++++++-----
 lib/notmuch-private.h                   |  2 ++
 lib/notmuch.h                           | 39 +++++++++++++++++++++++++
 lib/query.cc                            | 13 +++++++--
 test/T641-database-modified-messages.sh |  8 ++++-
 6 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 46638f80..ea815efe 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -212,6 +212,10 @@ _notmuch_message_create (const void *talloc_owner,
 	if (status)
 	    *status = NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND;
 	return NULL;
+    } catch (const Xapian::DatabaseModifiedError &error) {
+	if (status)
+	    *status = NOTMUCH_PRIVATE_STATUS_OPERATION_INVALIDATED;
+	return NULL;
     }
 
     return _notmuch_message_create_for_document (talloc_owner, notmuch,
diff --git a/lib/messages.c b/lib/messages.c
index eec0a162..727d64b9 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -79,6 +79,7 @@ _notmuch_messages_create (notmuch_message_list_t *list)
 
     messages->is_of_list_type = true;
     messages->iterator = list->head;
+    messages->status = NOTMUCH_STATUS_SUCCESS;
 
     return messages;
 }
@@ -98,16 +99,30 @@ _notmuch_messages_create (notmuch_message_list_t *list)
  *	   carefully control object construction with placement new
  *	   anyway. *sigh*
  */
+notmuch_status_t
+notmuch_messages_status (notmuch_messages_t *messages)
+{
+    bool valid;
+
+    if (messages == NULL)
+	return NOTMUCH_STATUS_ITERATOR_EXHAUSTED;
+
+    if (messages->status != NOTMUCH_STATUS_SUCCESS)
+	return messages->status;
+
+    if (! messages->is_of_list_type)
+	valid = _notmuch_mset_messages_valid (messages);
+    else
+	valid = messages->iterator != NULL;
+
+    return valid ?
+	NOTMUCH_STATUS_SUCCESS : NOTMUCH_STATUS_ITERATOR_EXHAUSTED;
+}
+
 notmuch_bool_t
 notmuch_messages_valid (notmuch_messages_t *messages)
 {
-    if (messages == NULL)
-	return false;
-
-    if (! messages->is_of_list_type)
-	return _notmuch_mset_messages_valid (messages);
-
-    return (messages->iterator != NULL);
+    return notmuch_messages_status(messages) == NOTMUCH_STATUS_SUCCESS;
 }
 
 bool
@@ -142,7 +157,7 @@ notmuch_messages_move_to_next (notmuch_messages_t *messages)
 	return;
     }
 
-    if (messages->iterator == NULL)
+    if (! notmuch_messages_valid(messages))
 	return;
 
     messages->iterator = messages->iterator->next;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 367e23e6..f4702dd8 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -147,6 +147,7 @@ typedef enum {
     NOTMUCH_PRIVATE_STATUS_NO_MAIL_ROOT				= NOTMUCH_STATUS_NO_MAIL_ROOT,
     NOTMUCH_PRIVATE_STATUS_BAD_QUERY_SYNTAX			= NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
     NOTMUCH_PRIVATE_STATUS_CLOSED_DATABASE			= NOTMUCH_STATUS_CLOSED_DATABASE,
+    NOTMUCH_PRIVATE_STATUS_OPERATION_INVALIDATED		= NOTMUCH_STATUS_OPERATION_INVALIDATED,
 
     /* Then add our own private values. */
     NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG		= NOTMUCH_STATUS_LAST_STATUS,
@@ -508,6 +509,7 @@ struct _notmuch_messages {
     bool is_of_list_type;
     notmuch_doc_id_set_t *excluded_doc_ids;
     notmuch_message_node_t *iterator;
+    notmuch_status_t status;
 };
 
 notmuch_message_list_t *
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 937fa24e..c9923da2 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -232,6 +232,20 @@ typedef enum {
      * Database is not fully opened, or has been closed
      */
     NOTMUCH_STATUS_CLOSED_DATABASE,
+    /**
+     * The iterator being examined has been exhausted and contains no more
+     * items.
+     */
+    NOTMUCH_STATUS_ITERATOR_EXHAUSTED,
+    /**
+     * An operation that was being performed on the database has been
+     * invalidated while in progress, and must be re-executed.
+     *
+     * This will typically happen while iterating over query results and the
+     * underlying Xapian database is modified by another process so that the
+     * currently open version cannot be read anymore.
+     */
+    NOTMUCH_STATUS_OPERATION_INVALIDATED,
     /**
      * Not an actual status value. Just a way to find out how many
      * valid status values there are.
@@ -1516,10 +1530,35 @@ notmuch_thread_destroy (notmuch_thread_t *thread);
  *
  * See the documentation of notmuch_query_search_messages for example
  * code showing how to iterate over a notmuch_messages_t object.
+ *
+ * Note that an iterator may become invalid either due to getting exhausted or
+ * due to a runtime error. Use notmuch_messages_status to distinguish
+ * between those cases.
  */
 notmuch_bool_t
 notmuch_messages_valid (notmuch_messages_t *messages);
 
+/**
+ * Get the status of the given 'messages' iterator.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: The iterator is valid; notmuch_messages_get will
+ *     return a valid object
+ *
+ * NOTMUCH_STATUS_ITERATOR_EXHAUSTED: All items have been read
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Iteration failed to allocate memory
+ *
+ * NOTMUCH_STATUS_OPERATION_INVALIDATED: Iteration was invalidated by the
+ *     database. Re-open the database and try again.
+ *
+ * See the documentation of notmuch_query_search_messages for example
+ * code showing how to iterate over a notmuch_messages_t object.
+ */
+notmuch_status_t
+notmuch_messages_status (notmuch_messages_t *messages);
+
 /**
  * Get the current message from 'messages' as a notmuch_message_t.
  *
diff --git a/lib/query.cc b/lib/query.cc
index 1c60c122..fda4e18f 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -371,6 +371,7 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 
 	messages->base.is_of_list_type = false;
 	messages->base.iterator = NULL;
+	messages->base.status = NOTMUCH_STATUS_SUCCESS;
 	messages->notmuch = notmuch;
 	new (&messages->iterator) Xapian::MSetIterator ();
 	new (&messages->iterator_end) Xapian::MSetIterator ();
@@ -509,9 +510,15 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages)
 				       mset_messages->notmuch, doc_id,
 				       &status);
 
-    if (message == NULL &&
-	status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
-	INTERNAL_ERROR ("a messages iterator contains a non-existent document ID.\n");
+    if (message == NULL) {
+	if (status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+	    INTERNAL_ERROR ("a messages iterator contains a non-existent document ID.\n");
+	} else if (status != NOTMUCH_PRIVATE_STATUS_SUCCESS) {
+	    messages->status = COERCE_STATUS(status, "error creating a message");
+	    return NULL;
+	}
+
+	INTERNAL_ERROR("NULL message with no error code\n");
     }
 
     if (messages->excluded_doc_ids &&
diff --git a/test/T641-database-modified-messages.sh b/test/T641-database-modified-messages.sh
index c95a4366..e223efc1 100755
--- a/test/T641-database-modified-messages.sh
+++ b/test/T641-database-modified-messages.sh
@@ -28,6 +28,7 @@ main (int argc, char **argv)
     notmuch_database_t *rw_db, *ro_db;
     notmuch_messages_t *messages_ro, *messages_rw;
     notmuch_query_t *query_ro, *query_rw;
+    notmuch_status_t status;
     char* msg = NULL;
 
     EXPECT0(notmuch_database_open_with_config (argv[1],
@@ -66,8 +67,12 @@ main (int argc, char **argv)
 	 notmuch_messages_move_to_next (messages_ro)) {
 	notmuch_message_t *message = notmuch_messages_get (messages_ro);
     }
+    status = notmuch_messages_status(messages_ro);
 
-    printf("SUCCESS\n");
+    if (status == NOTMUCH_STATUS_ITERATOR_EXHAUSTED)
+	printf("SUCCESS\n");
+    else
+	printf("status: %u\n", status);
     return 0;
 }
 EOF
@@ -77,6 +82,7 @@ cat <<'EOF' >EXPECTED
 SUCCESS
 == stderr ==
 EOF
+test_subtest_known_broken
 test_expect_equal_file EXPECTED OUTPUT
 
 test_done
-- 
2.39.5

_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: