[PATCH] tdb traverse transaction unlocking bug

Rusty Russell rusty at rustcorp.com.au
Thu Jul 16 09:22:51 MDT 2009


Recent playing with tdb revealed that the code which takes the transaction
lock for a traversal is buggy.  This has two causes:

1) Any (non-read) traversal inside a transaction causes the transaction lock
    to be released when the traversal finishes, and
2) Traversals inside traversals drop the transaction lock early (when the
    inner traverse finishes).

Fix is to do proper recursion with the transaction lock, like other locks.

The samba version of tdb has a different workaround in traverse.c (Volker's
patch of  May 20 2008), which solves the first problem, but not the second (and
this patch still applies, but then the code in traverse.c should be removed).

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

diff --git a/lib/tdb/common/lock.c b/lib/tdb/common/lock.c
index f156c0f..6f75787 100644
--- a/lib/tdb/common/lock.c
+++ b/lib/tdb/common/lock.c
@@ -301,7 +301,11 @@ int tdb_unlock(struct tdb_context *tdb, int list, int ltype)
  */
 int tdb_transaction_lock(struct tdb_context *tdb, int ltype)
 {
-	if (tdb->have_transaction_lock || tdb->global_lock.count) {
+	if (tdb->global_lock.count) {
+		return 0;
+	}
+	if (tdb->have_transaction_lock) {
+		tdb->have_transaction_lock++;
 		return 0;
 	}
 	if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, ltype, 
@@ -310,7 +314,7 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype)
 		tdb->ecode = TDB_ERR_LOCK;
 		return -1;
 	}
-	tdb->have_transaction_lock = 1;
+	tdb->have_transaction_lock++;
 	return 0;
 }
 
@@ -319,15 +323,13 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype)
  */
 int tdb_transaction_unlock(struct tdb_context *tdb)
 {
-	int ret;
-	if (!tdb->have_transaction_lock) {
+	if (tdb->global_lock.count) {
 		return 0;
 	}
-	ret = tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1);
-	if (ret == 0) {
-		tdb->have_transaction_lock = 0;
+	if (--tdb->have_transaction_lock) {
+		return 0;
 	}
-	return ret;
+	return tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1);
 }
 
 
diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h
index ffac89f..c2f51db 100644
--- a/lib/tdb/common/tdb_private.h
+++ b/lib/tdb/common/tdb_private.h
@@ -166,7 +168,7 @@ struct tdb_context {
 	struct tdb_transaction *transaction;
 	int page_size;
 	int max_dead_records;
-	bool have_transaction_lock;
+	int have_transaction_lock;
 	volatile sig_atomic_t *interrupt_sig_ptr;
 };
 



More information about the samba-technical mailing list