From bfe4555325086d5cb99e3962424128f94d1c921d Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sat, 11 Jun 2011 00:07:54 -0400 Subject: [PATCH] lib: Remove message document directly after removing the last file name. Previously, notmuch_database_remove_message would remove the message file name, sync the change to the message document, re-find the message document, and then delete it if there were no more file names. An interruption after sync'ing would result in a file-name-less, permanently un-removable zombie message that would produce errors and odd results in searches. We could wrap this in an atomic section, but it's much simpler to eliminate the round-about approach and just delete the message document instead of sync'ing it if we removed the last filename. --- lib/database.cc | 25 +++++-------------------- lib/message.cc | 16 ++++++++++++++++ lib/notmuch-private.h | 3 +++ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 92c3c4e0..cf87d08d 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -1747,7 +1747,6 @@ notmuch_status_t notmuch_database_remove_message (notmuch_database_t *notmuch, const char *filename) { - Xapian::WritableDatabase *db; void *local; const char *prefix = _find_prefix ("file-direntry"); char *direntry, *term; @@ -1761,8 +1760,6 @@ notmuch_database_remove_message (notmuch_database_t *notmuch, local = talloc_new (notmuch); - db = static_cast (notmuch->xapian_db); - try { status = _notmuch_database_filename_to_direntry (local, notmuch, @@ -1785,23 +1782,11 @@ notmuch_database_remove_message (notmuch_database_t *notmuch, return COERCE_STATUS (private_status, "Inconsistent document ID in datbase."); - _notmuch_message_remove_filename (message, filename); - _notmuch_message_sync (message); - - /* Take care to find document after sync'ing filename removal. */ - document = find_document_for_doc_id (notmuch, *i); - j = document.termlist_begin (); - j.skip_to (prefix); - - /* Was this the last file-direntry in the message? */ - if (j == document.termlist_end () || - strncmp ((*j).c_str (), prefix, strlen (prefix))) - { - db->delete_document (document.get_docid ()); - status = NOTMUCH_STATUS_SUCCESS; - } else { - status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; - } + status = _notmuch_message_remove_filename (message, filename); + if (status == NOTMUCH_STATUS_SUCCESS) + _notmuch_message_delete (message); + else if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) + _notmuch_message_sync (message); } } catch (const Xapian::Error &error) { fprintf (stderr, "Error: A Xapian exception occurred removing message: %s\n", diff --git a/lib/message.cc b/lib/message.cc index 20b77580..531d3043 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -822,6 +822,22 @@ _notmuch_message_sync (notmuch_message_t *message) db->replace_document (message->doc_id, message->doc); } +/* Delete a message document from the database. */ +notmuch_status_t +_notmuch_message_delete (notmuch_message_t *message) +{ + notmuch_status_t status; + Xapian::WritableDatabase *db; + + status = _notmuch_database_ensure_writable (message->notmuch); + if (status) + return status; + + db = static_cast (message->notmuch->xapian_db); + db->delete_document (message->doc_id); + return NOTMUCH_STATUS_SUCCESS; +} + /* Ensure that 'message' is not holding any file object open. Future * calls to various functions will still automatically open the * message file as needed. diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h index 02e24ee8..d3195305 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -293,6 +293,9 @@ _notmuch_message_set_date (notmuch_message_t *message, void _notmuch_message_sync (notmuch_message_t *message); +notmuch_status_t +_notmuch_message_delete (notmuch_message_t *message); + void _notmuch_message_close (notmuch_message_t *message); -- 2.26.2