From: Austin Clements <aclements@csail.mit.edu>
To: notmuch@notmuchmail.org
Subject: [PATCH v3] lib: Simplify close and codify aborting atomic section
Date: Wed, 24 Sep 2014 17:32:50 -0400
Message-Id: <1411594370-9794-1-git-send-email-aclements@csail.mit.edu>
In-Reply-To: <20140924212839.GE20130@odin.tremily.us>
References: <20140924212839.GE20130@odin.tremily.us>
Precedence: list

From: Austin Clements <amdragon@mit.edu>

In Xapian, closing a database implicitly aborts any outstanding
transaction and commits changes.  For historical reasons,
notmuch_database_close had grown to almost, but not quite duplicate
this behavior.  Before closing the database, it would explicitly (and
unnecessarily) commit it.  However, if there was an outstanding
transaction (ie atomic section), commit would throw a Xapian
exception, which notmuch_database_close would unnecessarily print to
stderr, even though notmuch_database_close would ultimately abort the
transaction anyway when it called close.

This patch simplifies notmuch_database_close to just call
Database::close.  This works for both read-only and read/write
databases, takes care of committing changes, unifies the exception
handling path, and codifies aborting outstanding transactions.  This
is currently the only way to abort an atomic section (and may remain
so, since it would be difficult to roll back things we may have cached
from rolled-back modifications).
---

'Doh.  Left out the all-critical "git commit" before sending v2.

 lib/database.cc | 23 +++++++----------------
 lib/notmuch.h   |  8 +++++++-
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index a3a7cd3..1f7ff2a 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -903,28 +903,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 {
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 
-    try {
-	if (notmuch->xapian_db != NULL &&
-	    notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
-	    (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();
-    } catch (const Xapian::Error &error) {
-	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-	if (! notmuch->exception_reported) {
-	    fprintf (stderr, "Error: A Xapian exception occurred flushing database: %s\n",
-		     error.get_msg().c_str());
-	}
-    }
-
     /* Many Xapian objects (and thus notmuch objects) hold references to
      * the database, so merely deleting the database may not suffice to
-     * close it.  Thus, we explicitly close it here. */
+     * close it.  Thus, we explicitly close it here.  This will
+     * implicitly abort any outstanding transaction and commit changes. */
     if (notmuch->xapian_db != NULL) {
 	try {
 	    notmuch->xapian_db->close();
 	} catch (const Xapian::Error &error) {
-	    /* don't clobber previous error status */
-	    if (status == NOTMUCH_STATUS_SUCCESS)
-		status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+	    status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+	    if (! notmuch->exception_reported) {
+		fprintf (stderr, "Error: A Xapian exception occurred closing database: %s\n",
+			 error.get_msg().c_str());
+	    }
 	}
     }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index fe2340b..49a3c79 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -281,7 +281,7 @@ notmuch_database_open (const char *path,
 		       notmuch_database_t **database);
 
 /**
- * Close the given notmuch database.
+ * Commit changes and close the given notmuch database.
  *
  * After notmuch_database_close has been called, calls to other
  * functions on objects derived from this database may either behave
@@ -292,6 +292,12 @@ notmuch_database_open (const char *path,
  * notmuch_database_close can be called multiple times.  Later calls
  * have no effect.
  *
+ * If the caller is currently in an atomic section (there was a
+ * notmuch_database_begin_atomic without a matching
+ * notmuch_database_end_atomic), this will abort the atomic section,
+ * discarding any modifications made in the atomic section.  All
+ * changes up to this will be committed.
+ *
  * Return value:
  *
  * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
-- 
2.1.0