Package: libxapian30
Version: 1.4.3-2+deb9u1
Severity: important
Tags: patch upstream

If changes to a new database which don't modify the termlist table
are committed, then a block which has been allocated to be the root
block in the termlist table gets leaked.  This case is triggered by
notmuch - the result is a database which is slightly larger than
it would otherwise be, but works fine *except* that consistency
checking with xapian-check/Database::check() detects there's an
unused block not on the freelist and reports this as
"DatabaseCorruptError" - this tends to alarm users.

While tracking down the above problem, I found a second case where
blocks can be leaked when cancel_transaction() is called.

Both of these were fixed in upstream xapian-core 1.4.7 - the combined
patch is attached.

Cheers,
    Olly
diff --git a/xapian-core/backends/glass/glass_table.cc b/xapian-core/backends/glass/glass_table.cc
index 7342f7496c48..1669431b6c6c 100644
--- a/xapian-core/backends/glass/glass_table.cc
+++ b/xapian-core/backends/glass/glass_table.cc
@@ -1639,6 +1639,7 @@ GlassTable::read_root()
 	    /* writing - */
 	    SET_REVISION(p, revision_number + 1);
 	    C[0].set_n(free_list.get_block(this, block_size));
+	    C[0].rewrite = true;
 	}
     } else {
 	/* using a root block stored on disk */
@@ -1853,9 +1854,7 @@ GlassTable::flush_db()
 	}
     }
 
-    if (Btree_modified) {
-	faked_root_block = false;
-    }
+    faked_root_block = false;
 }
 
 void
@@ -1944,6 +1943,13 @@ GlassTable::cancel(const RootInfo & root_info, glass_revision_number_t rev)
     item_count =       root_info.get_num_entries();
     faked_root_block = root_info.get_root_is_fake();
     sequential =       root_info.get_sequential();
+    const string & fl_serialised = root_info.get_free_list();
+    if (!fl_serialised.empty()) {
+	if (!free_list.unpack(fl_serialised))
+	    throw Xapian::DatabaseCorruptError("Bad freelist metadata");
+    } else {
+	free_list.reset();
+    }
 
     Btree_modified = false;
 
diff --git a/xapian-core/tests/api_backend.cc b/xapian-core/tests/api_backend.cc
index a520be112286..a6d97742865f 100644
--- a/xapian-core/tests/api_backend.cc
+++ b/xapian-core/tests/api_backend.cc
@@ -1666,3 +1666,23 @@ DEFINE_TESTCASE(checkatleast4, backend) {
     TEST_EQUAL(mset.size(), 0);
     return true;
 }
+
+/// Regression test for glass bug fixed in 1.4.6 and 1.5.0.
+DEFINE_TESTCASE(nodocs1, transactions && !remote) {
+    {
+	Xapian::WritableDatabase db = get_named_writable_database("nodocs1");
+	db.set_metadata("foo", "bar");
+	db.commit();
+	Xapian::Document doc;
+	doc.add_term("baz");
+	db.add_document(doc);
+	db.commit();
+    }
+
+    size_t check_errors =
+	Xapian::Database::check(get_named_writable_database_path("nodocs1"),
+				Xapian::DBCHECK_SHOW_STATS, &tout);
+    TEST_EQUAL(check_errors, 0);
+
+    return true;
+}
diff --git a/xapian-core/tests/api_transdb.cc b/xapian-core/tests/api_transdb.cc
index eda0a6ef5936..32eede143f5a 100644
--- a/xapian-core/tests/api_transdb.cc
+++ b/xapian-core/tests/api_transdb.cc
@@ -1,7 +1,7 @@
 /** @file api_transdb.cc
  * @brief tests requiring a database backend supporting transactions
  */
-/* Copyright (C) 2006,2009 Olly Betts
+/* Copyright (C) 2006,2009,2018 Olly Betts
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -126,3 +126,24 @@ DEFINE_TESTCASE(canceltransaction2, transactions) {
 
     return true;
 }
+
+/// Regression test for glass bug fixed in 1.4.6 and 1.5.0.
+DEFINE_TESTCASE(canceltransaction3, transactions && !remote) {
+    {
+	Xapian::WritableDatabase db = get_named_writable_database("canceltransaction3");
+	db.begin_transaction();
+	Xapian::Document doc;
+	doc.add_term("baz");
+	db.add_document(doc);
+	db.cancel_transaction();
+	db.add_document(doc);
+	db.commit();
+    }
+
+    size_t check_errors =
+	Xapian::Database::check(get_named_writable_database_path("canceltransaction3"),
+				Xapian::DBCHECK_SHOW_STATS, &tout);
+    TEST_EQUAL(check_errors, 0);
+
+    return true;
+}

Attachment: signature.asc
Description: PGP signature

Reply via email to