Browse Source

tdb2: fix two transaction bugs.

One but were we didn't update the map_size if we expanded the
transaction but didn't create a new recovery area (most easily
reproduced by setting the TDB_NOSYNC flag).

Another out-by-one bug in transaction_direct where we would give
read-access to the underlying file despite the last block having been
modified.

Both these were found by tdbtorture.
Rusty Russell 15 years ago
parent
commit
e1fd1d9623
2 changed files with 48 additions and 2 deletions
  1. 45 0
      ccan/tdb2/test/run-expand-in-transaction.c
  2. 3 2
      ccan/tdb2/transaction.c

+ 45 - 0
ccan/tdb2/test/run-expand-in-transaction.c

@@ -0,0 +1,45 @@
+#include <ccan/tdb2/tdb.c>
+#include <ccan/tdb2/open.c>
+#include <ccan/tdb2/free.c>
+#include <ccan/tdb2/lock.c>
+#include <ccan/tdb2/io.c>
+#include <ccan/tdb2/hash.c>
+#include <ccan/tdb2/check.c>
+#include <ccan/tdb2/transaction.c>
+#include <ccan/tap/tap.h>
+#include "logging.h"
+
+int main(int argc, char *argv[])
+{
+	unsigned int i;
+	struct tdb_context *tdb;
+	int flags[] = { TDB_DEFAULT, TDB_NOMMAP,
+			TDB_CONVERT, TDB_NOMMAP|TDB_CONVERT,
+			TDB_CONVERT|TDB_NOSYNC,
+			TDB_NOMMAP|TDB_CONVERT|TDB_NOSYNC };
+	struct tdb_data key = tdb_mkdata("key", 3);
+	struct tdb_data data = tdb_mkdata("data", 4);
+
+	plan_tests(sizeof(flags) / sizeof(flags[0]) * 7 + 1);
+
+	for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) {
+		size_t size;
+		tdb = tdb_open("run-expand-in-transaction.tdb", flags[i],
+			       O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr);
+		ok1(tdb);
+		if (!tdb)
+			continue;
+
+		size = tdb->file->map_size;
+		ok1(tdb_transaction_start(tdb) == 0);
+		ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
+		ok1(tdb->file->map_size > size);
+		ok1(tdb_transaction_commit(tdb) == 0);
+		ok1(tdb->file->map_size > size);
+		ok1(tdb_check(tdb, NULL, NULL) == 0);
+		tdb_close(tdb);
+	}
+
+	ok1(tap_log_messages == 0);
+	return exit_status();
+}

+ 3 - 2
ccan/tdb2/transaction.c

@@ -402,7 +402,7 @@ static void *transaction_direct(struct tdb_context *tdb, tdb_off_t off,
 		return tdb->transaction->blocks[blk] + off % getpagesize();
 		return tdb->transaction->blocks[blk] + off % getpagesize();
 
 
 	/* Otherwise must be all not copied. */
 	/* Otherwise must be all not copied. */
-	while (blk < end_blk) {
+	while (blk <= end_blk) {
 		if (blk >= tdb->transaction->num_blocks)
 		if (blk >= tdb->transaction->num_blocks)
 			break;
 			break;
 		if (tdb->transaction->blocks[blk])
 		if (tdb->transaction->blocks[blk])
@@ -1080,7 +1080,8 @@ enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb)
 #endif
 #endif
 
 
 	/* use a transaction cancel to free memory and remove the
 	/* use a transaction cancel to free memory and remove the
-	   transaction locks */
+	   transaction locks: it "restores" map_size, too. */
+	tdb->transaction->old_map_size = tdb->file->map_size;
 	_tdb_transaction_cancel(tdb);
 	_tdb_transaction_cancel(tdb);
 
 
 	return tdb->last_error = TDB_SUCCESS;
 	return tdb->last_error = TDB_SUCCESS;