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; +}
signature.asc
Description: PGP signature