While a number of errors can happen during thread iteration (DatabaseModifiedError, memory allocation errors, etc.), threads iteration cannot signal them to the caller, and either triggers an internal error (aborting the aller) or returns NULL from notmuch_threads_get() with no information on what actually went wrong. Add a new public function notmuch_threads_status() - similar to previously added notmuch_messages_status() - that allows propagating those errors to the caller. Use this to remove the INTERNAL_ERROR() in _notmuch_thread_create() (triggered by T642). Fixes: https://github.com/pazz/alot/issues/1460 --- lib/notmuch-private.h | 3 +- lib/notmuch.h | 31 ++++++++++++++++++-- lib/query.cc | 40 ++++++++++++++++++++------ lib/thread.cc | 10 ++++--- test/T642-database-modified-threads.sh | 33 +++++++++++++++++---- 5 files changed, 94 insertions(+), 23 deletions(-) diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h index db49e77d..ebb38a5c 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -714,7 +714,8 @@ _notmuch_thread_create (void *ctx, notmuch_doc_id_set_t *match_set, notmuch_string_list_t *excluded_terms, notmuch_exclude_t omit_exclude, - notmuch_sort_t sort); + notmuch_sort_t sort, + notmuch_private_status_t *status); /* indexopts.c */ diff --git a/lib/notmuch.h b/lib/notmuch.h index e634e41f..10a1fc6e 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -1134,7 +1134,7 @@ notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag); * * for (stat = notmuch_query_search_threads (query, &threads); * stat == NOTMUCH_STATUS_SUCCESS && - * notmuch_threads_valid (threads); + * ! notmuch_threads_status (threads); * notmuch_threads_move_to_next (threads)) * { * thread = notmuch_threads_get (threads); @@ -1259,10 +1259,35 @@ notmuch_query_destroy (notmuch_query_t *query); * * See the documentation of notmuch_query_search_threads for example * code showing how to iterate over a notmuch_threads_t object. + * + * Note that an iterator may become invalid either due to getting exhausted or + * due to a runtime error. Use notmuch_threads_status to distinguish + * between those cases. */ notmuch_bool_t notmuch_threads_valid (notmuch_threads_t *threads); +/** + * Get the status of the given 'threads' iterator. + * + * Return value: + * + * NOTMUCH_STATUS_SUCCESS: The iterator is valid; notmuch_threads_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_threads for example + * code showing how to iterate over a notmuch_threads_t object. + */ +notmuch_status_t +notmuch_threads_status (notmuch_threads_t *threads); + /** * Get the current thread from 'threads' as a notmuch_thread_t. * @@ -1283,8 +1308,8 @@ notmuch_threads_get (notmuch_threads_t *threads); * * If 'threads' is already pointing at the last thread then the * iterator will be moved to a point just beyond that last thread, - * (where notmuch_threads_valid will return FALSE and - * notmuch_threads_get will return NULL). + * (where notmuch_threads_status will return NOTMUCH_STATUS_ITERATOR_EXHAUSTED + * and notmuch_threads_get will return NULL). * * See the documentation of notmuch_query_search_threads for example * code showing how to iterate over a notmuch_threads_t object. diff --git a/lib/query.cc b/lib/query.cc index 976fe76d..56d1509d 100644 --- a/lib/query.cc +++ b/lib/query.cc @@ -61,6 +61,7 @@ struct _notmuch_threads { /* The set of matched docid's that have not been assigned to a * thread. Initially, this contains every docid in doc_ids. */ notmuch_doc_id_set_t match_set; + notmuch_status_t status; }; /* We need this in the message functions so forward declare. */ @@ -611,6 +612,7 @@ notmuch_query_search_threads (notmuch_query_t *query, threads = talloc (query, notmuch_threads_t); if (threads == NULL) return NOTMUCH_STATUS_OUT_OF_MEMORY; + threads->status = NOTMUCH_STATUS_SUCCESS; threads->doc_ids = NULL; talloc_set_destructor (threads, _notmuch_threads_destructor); @@ -650,11 +652,19 @@ notmuch_query_destroy (notmuch_query_t *query) notmuch_bool_t notmuch_threads_valid (notmuch_threads_t *threads) +{ + return notmuch_threads_status(threads) == NOTMUCH_STATUS_SUCCESS; +} + +notmuch_status_t +notmuch_threads_status (notmuch_threads_t *threads) { unsigned int doc_id; if (! threads) - return false; + return NOTMUCH_STATUS_ITERATOR_EXHAUSTED; + if (threads->status) + return threads->status; while (threads->doc_id_pos < threads->doc_ids->len) { doc_id = g_array_index (threads->doc_ids, unsigned int, @@ -665,26 +675,38 @@ notmuch_threads_valid (notmuch_threads_t *threads) threads->doc_id_pos++; } - return threads->doc_id_pos < threads->doc_ids->len; + return (threads->doc_id_pos < threads->doc_ids->len) ? + NOTMUCH_STATUS_SUCCESS : NOTMUCH_STATUS_ITERATOR_EXHAUSTED; } + notmuch_thread_t * notmuch_threads_get (notmuch_threads_t *threads) { + notmuch_thread_t *thread; unsigned int doc_id; + notmuch_private_status_t status; if (! notmuch_threads_valid (threads)) return NULL; doc_id = g_array_index (threads->doc_ids, unsigned int, threads->doc_id_pos); - return _notmuch_thread_create (threads->query, - threads->query->notmuch, - doc_id, - &threads->match_set, - threads->query->exclude_terms, - threads->query->omit_excluded, - threads->query->sort); + thread = _notmuch_thread_create (threads->query, + threads->query->notmuch, + doc_id, + &threads->match_set, + threads->query->exclude_terms, + threads->query->omit_excluded, + threads->query->sort, + &status); + if (thread == NULL) { + if (! status) + INTERNAL_ERROR ("NULL thread with no error code"); + threads->status = COERCE_STATUS (status, "error creating a thread"); + } + + return thread; } void diff --git a/lib/thread.cc b/lib/thread.cc index 168a9e8b..f286be31 100644 --- a/lib/thread.cc +++ b/lib/thread.cc @@ -553,7 +553,8 @@ _notmuch_thread_create (void *ctx, notmuch_doc_id_set_t *match_set, notmuch_string_list_t *exclude_terms, notmuch_exclude_t omit_excluded, - notmuch_sort_t sort) + notmuch_sort_t sort, + notmuch_private_status_t *status_p) { void *local = talloc_new (ctx); notmuch_thread_t *thread = NULL; @@ -566,9 +567,9 @@ _notmuch_thread_create (void *ctx, notmuch_message_t *message; notmuch_status_t status; - seed_message = _notmuch_message_create (local, notmuch, seed_doc_id, NULL); - if (! seed_message) - INTERNAL_ERROR ("Thread seed message %u does not exist", seed_doc_id); + seed_message = _notmuch_message_create (local, notmuch, seed_doc_id, status_p); + if (*status_p) + return NULL; thread_id = notmuch_message_get_thread_id (seed_message); thread_id_query_string = talloc_asprintf (local, "thread:%s", thread_id); @@ -659,6 +660,7 @@ _notmuch_thread_create (void *ctx, DONE: talloc_free (local); + *status_p = NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY; return thread; } diff --git a/test/T642-database-modified-threads.sh b/test/T642-database-modified-threads.sh index 1717cd39..cba6aaa0 100755 --- a/test/T642-database-modified-threads.sh +++ b/test/T642-database-modified-threads.sh @@ -11,7 +11,6 @@ add_email_corpus cp -a $NOTMUCH_SRCDIR/test/corpora/lkml ${MAIL_DIR}/ test_begin_subtest "handling NOTMUCH_STATUS_OPERATION_INVALIDATED in _notmuch_thread_create" -test_subtest_known_broken test_C ${MAIL_DIR} <<EOF #include <notmuch-test.h> @@ -27,6 +26,7 @@ main (int argc, char **argv) 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, @@ -59,13 +59,34 @@ main (int argc, char **argv) notmuch_database_close (rw_db); - for (; - notmuch_threads_valid (threads); - notmuch_threads_move_to_next (threads)) { - notmuch_thread_t *thread = notmuch_threads_get (threads); + // 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_threads_valid (threads); + notmuch_threads_move_to_next (threads)) { + notmuch_thread_t *thread = notmuch_threads_get (threads); + } + status = notmuch_threads_status (threads); + 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_threads (query_ro, &threads)); } - 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