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.
---
lib/database.cc | 4 ++
lib/message.cc | 4 ++
lib/messages.c | 31 +++++++++++----
lib/notmuch-private.h | 3 ++
lib/notmuch.h | 53 ++++++++++++++++++++++---
lib/query.cc | 13 ++++--
test/T641-database-modified-messages.sh | 34 +++++++++++++---
7 files changed, 120 insertions(+), 22 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 737a3f30..8f687eee 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -313,6 +313,10 @@ notmuch_status_to_string (notmuch_status_t status)
return "Syntax error in query";
case NOTMUCH_STATUS_NO_MAIL_ROOT:
return "No mail root found";
+ case NOTMUCH_STATUS_ITERATOR_EXHAUSTED:
+ return "Iterator exhausted";
+ case NOTMUCH_STATUS_OPERATION_INVALIDATED:
+ return "Operation invalidated due to concurrent database modification";
default:
case NOTMUCH_STATUS_LAST_STATUS:
return "Unknown error status value";
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..619b1d2f 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..db49e77d 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -147,6 +147,8 @@ 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_ITERATOR_EXHAUSTED = NOTMUCH_STATUS_ITERATOR_EXHAUSTED,
+ 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 +510,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..e634e41f 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.
@@ -1177,7 +1191,7 @@ notmuch_query_search_threads_st (notmuch_query_t *query, notmuch_threads_t **out
* return EXIT_FAILURE;
*
* for (;
- * notmuch_messages_valid (messages);
+ * ! notmuch_messages_status (messages);
* notmuch_messages_move_to_next (messages))
* {
* message = notmuch_messages_get (messages);
@@ -1185,6 +1199,9 @@ notmuch_query_search_threads_st (notmuch_query_t *query, notmuch_threads_t **out
* notmuch_message_destroy (message);
* }
*
+ * if (notmuch_messages_status (messages) != NOTMUCH_STATUS_ITERATOR_EXHAUSTED)
+ * return EXIT_FAILURE;
+ *
* notmuch_query_destroy (query);
*
* Note: If you are finished with a message before its containing
@@ -1516,10 +1533,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.
*
@@ -1540,8 +1582,8 @@ notmuch_messages_get (notmuch_messages_t *messages);
*
* If 'messages' is already pointing at the last message then the
* iterator will be moved to a point just beyond that last message,
- * (where notmuch_messages_valid will return FALSE and
- * notmuch_messages_get will return NULL).
+ * (where notmuch_messages_status will return NOTMUCH_STATUS_ITERATOR_EXHAUSTED
+ * and notmuch_messages_get will return NULL).
*
* See the documentation of notmuch_query_search_messages for example
* code showing how to iterate over a notmuch_messages_t object.
@@ -1627,8 +1669,9 @@ notmuch_message_get_thread_id (notmuch_message_t *message);
* will return NULL.
*
* If there are no replies to 'message', this function will return
- * NULL. (Note that notmuch_messages_valid will accept that NULL
- * value as legitimate, and simply return FALSE for it.)
+ * NULL. (Note that notmuch_messages_status will accept that NULL
+ * value as legitimate, and simply return NOTMUCH_STATUS_ITERATOR_EXHAUSTED
+ * for it.)
*
* This function also returns NULL if it triggers a Xapian exception.
*
diff --git a/lib/query.cc b/lib/query.cc
index 1c60c122..976fe76d 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 200a950d..da0557c6 100755
--- a/test/T641-database-modified-messages.sh
+++ b/test/T641-database-modified-messages.sh
@@ -16,7 +16,6 @@ add_email_corpus
cp -a $NOTMUCH_SRCDIR/test/corpora/lkml ${MAIL_DIR}/
test_begin_subtest "catching DatabaseModifiedError in _notmuch_message_create"
-test_subtest_known_broken
test_C ${MAIL_DIR} <<EOF
#include <notmuch-test.h>
@@ -29,7 +28,9 @@ 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;
+ unsigned try;
EXPECT0 (notmuch_database_open_with_config (argv[1],
NOTMUCH_DATABASE_MODE_READ_ONLY,
@@ -62,13 +63,34 @@ main (int argc, char **argv)
notmuch_database_close (rw_db);
- for (;
- notmuch_messages_valid (messages_ro);
- notmuch_messages_move_to_next (messages_ro)) {
- notmuch_message_t *message = notmuch_messages_get (messages_ro);
+ // try iterating over the query up to twice, we expect a Xapian
+ // DatabaseModifiedError (mapped to NOTMUCH_STATUS_OPERATION_INVALIDATED)
+ // on the first try
+ for (try = 0; try < 2; try++) {
+ for (;
+ notmuch_messages_valid (messages_ro);
+ notmuch_messages_move_to_next (messages_ro)) {
+ notmuch_message_t *message = notmuch_messages_get (messages_ro);
+ }
+ status = notmuch_messages_status (messages_ro);
+ if (status != NOTMUCH_STATUS_OPERATION_INVALIDATED)
+ break;
+
+ notmuch_query_destroy (query_ro);
+ notmuch_database_close (ro_db);
+
+ EXPECT0 (notmuch_database_open_with_config (argv[1],
+ NOTMUCH_DATABASE_MODE_READ_ONLY,
+ "", NULL, &ro_db, &msg));
+ query_ro = notmuch_query_create (ro_db, "");
+ assert (query_ro);
+ EXPECT0 (notmuch_query_search_messages (query_ro, &messages_ro));
}
- printf ("SUCCESS\n");
+ if (status == NOTMUCH_STATUS_ITERATOR_EXHAUSTED)
+ printf ("SUCCESS\n");
+ else
+ printf ("status: %s\n", notmuch_status_to_string (status));
return 0;
}
EOF
--
2.39.5
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org