From 0868b7c77d42efd5f361f605bfc0d8d46841db95 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 16 Sep 2005 03:52:42 +0000 Subject: r10253: a fairly large tdb cleanup and re-organise. Nearly all of this change just involves splitting up the core tdb.c code into separate files on logical boundaries, but there are some minor functional changes as well: - move the 'struct tdb_context' into tdb_private.h, hiding it from users. This was done to allow the structure to change without breaking code that uses tdb. - added accessor functions tdb_fd(), tdb_name(), and tdb_log_fn() to access the elements of struct tdb_context that were used by external code but are no longer visible - simplied tdb_append() to use tdb_fetch()/tdb_store(), which is just as good due to the way tdb locks work - changed some of the types (such as tdb_off to tdb_off_t) to make syntax highlighting work better - removed the old optional spinlock code. It was a bad idea. - fixed a bug in tdb_reopen_all() that caused tdbtorture to sometimes fail or report nasty looking errors. This is the only real bug fixed in this commit. Jeremy/Jerry, you might like to pickup this change for Samba3, as that could definately affect smbd in Samba3. The aim of all of these changes is to make the tdb transactions/journaling code I am working on easier to write. I started to write it on top of the existing tdb.c code and it got very messy. Splitting up the code makes it much easier to follow. There are more cleanups we could do in tdb, such as using uint32_t instead of u32 (suggested by metze). I'll leave those for another day. (This used to be commit 4673cdd0d261614e707b72a7a348bb0e7dbb2482) --- source4/lib/tdb/common/traverse.c | 274 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 274 insertions(+) create mode 100644 source4/lib/tdb/common/traverse.c (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c new file mode 100644 index 0000000000..d14be355a2 --- /dev/null +++ b/source4/lib/tdb/common/traverse.c @@ -0,0 +1,274 @@ + /* + Unix SMB/CIFS implementation. + + trivial database library + + Copyright (C) Andrew Tridgell 1999-2005 + Copyright (C) Paul `Rusty' Russell 2000 + Copyright (C) Jeremy Allison 2000-2003 + + ** NOTE! The following LGPL license applies to the tdb + ** library. This does NOT imply that all of Samba is released + ** under the LGPL + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2 of the License, or (at your option) any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with this library; if not, write to the Free Software + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +*/ + +#include "tdb_private.h" + +/* Uses traverse lock: 0 = finish, -1 = error, other = record offset */ +static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tlock, + struct list_struct *rec) +{ + int want_next = (tlock->off != 0); + + /* Lock each chain from the start one. */ + for (; tlock->hash < tdb->header.hash_size; tlock->hash++) { + if (!tlock->off && tlock->hash != 0) { + /* this is an optimisation for the common case where + the hash chain is empty, which is particularly + common for the use of tdb with ldb, where large + hashes are used. In that case we spend most of our + time in tdb_brlock(), locking empty hash chains. + + To avoid this, we do an unlocked pre-check to see + if the hash chain is empty before starting to look + inside it. If it is empty then we can avoid that + hash chain. If it isn't empty then we can't believe + the value we get back, as we read it without a + lock, so instead we get the lock and re-fetch the + value below. + + Notice that not doing this optimisation on the + first hash chain is critical. We must guarantee + that we have done at least one fcntl lock at the + start of a search to guarantee that memory is + coherent on SMP systems. If records are added by + others during the search then thats OK, and we + could possibly miss those with this trick, but we + could miss them anyway without this trick, so the + semantics don't change. + + With a non-indexed ldb search this trick gains us a + factor of around 80 in speed on a linux 2.6.x + system (testing using ldbtest). + */ + tdb_next_hash_chain(tdb, &tlock->hash); + if (tlock->hash == tdb->header.hash_size) { + continue; + } + } + + if (tdb_lock(tdb, tlock->hash, F_WRLCK) == -1) + return -1; + + /* No previous record? Start at top of chain. */ + if (!tlock->off) { + if (tdb_ofs_read(tdb, TDB_HASH_TOP(tlock->hash), + &tlock->off) == -1) + goto fail; + } else { + /* Otherwise unlock the previous record. */ + if (tdb_unlock_record(tdb, tlock->off) != 0) + goto fail; + } + + if (want_next) { + /* We have offset of old record: grab next */ + if (tdb_rec_read(tdb, tlock->off, rec) == -1) + goto fail; + tlock->off = rec->next; + } + + /* Iterate through chain */ + while( tlock->off) { + tdb_off_t current; + if (tdb_rec_read(tdb, tlock->off, rec) == -1) + goto fail; + + /* Detect infinite loops. From "Shlomi Yaakobovich" . */ + if (tlock->off == rec->next) { + TDB_LOG((tdb, 0, "tdb_next_lock: loop detected.\n")); + goto fail; + } + + if (!TDB_DEAD(rec)) { + /* Woohoo: we found one! */ + if (tdb_lock_record(tdb, tlock->off) != 0) + goto fail; + return tlock->off; + } + + /* Try to clean dead ones from old traverses */ + current = tlock->off; + tlock->off = rec->next; + if (!tdb->read_only && + tdb_do_delete(tdb, current, rec) != 0) + goto fail; + } + tdb_unlock(tdb, tlock->hash, F_WRLCK); + want_next = 0; + } + /* We finished iteration without finding anything */ + return TDB_ERRCODE(TDB_SUCCESS, 0); + + fail: + tlock->off = 0; + if (tdb_unlock(tdb, tlock->hash, F_WRLCK) != 0) + TDB_LOG((tdb, 0, "tdb_next_lock: On error unlock failed!\n")); + return -1; +} + +/* traverse the entire database - calling fn(tdb, key, data) on each element. + return -1 on error or the record count traversed + if fn is NULL then it is not called + a non-zero return value from fn() indicates that the traversal should stop + */ +int tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *private) +{ + TDB_DATA key, dbuf; + struct list_struct rec; + struct tdb_traverse_lock tl = { NULL, 0, 0 }; + int ret, count = 0; + + /* This was in the initializaton, above, but the IRIX compiler + * did not like it. crh + */ + tl.next = tdb->travlocks.next; + + /* fcntl locks don't stack: beware traverse inside traverse */ + tdb->travlocks.next = &tl; + + /* tdb_next_lock places locks on the record returned, and its chain */ + while ((ret = tdb_next_lock(tdb, &tl, &rec)) > 0) { + count++; + /* now read the full record */ + key.dptr = tdb_alloc_read(tdb, tl.off + sizeof(rec), + rec.key_len + rec.data_len); + if (!key.dptr) { + ret = -1; + if (tdb_unlock(tdb, tl.hash, F_WRLCK) != 0) + goto out; + if (tdb_unlock_record(tdb, tl.off) != 0) + TDB_LOG((tdb, 0, "tdb_traverse: key.dptr == NULL and unlock_record failed!\n")); + goto out; + } + key.dsize = rec.key_len; + dbuf.dptr = key.dptr + rec.key_len; + dbuf.dsize = rec.data_len; + + /* Drop chain lock, call out */ + if (tdb_unlock(tdb, tl.hash, F_WRLCK) != 0) { + ret = -1; + goto out; + } + if (fn && fn(tdb, key, dbuf, private)) { + /* They want us to terminate traversal */ + ret = count; + if (tdb_unlock_record(tdb, tl.off) != 0) { + TDB_LOG((tdb, 0, "tdb_traverse: unlock_record failed!\n"));; + ret = -1; + } + tdb->travlocks.next = tl.next; + SAFE_FREE(key.dptr); + return count; + } + SAFE_FREE(key.dptr); + } +out: + tdb->travlocks.next = tl.next; + if (ret < 0) + return -1; + else + return count; +} + +/* find the first entry in the database and return its key */ +TDB_DATA tdb_firstkey(struct tdb_context *tdb) +{ + TDB_DATA key; + struct list_struct rec; + + /* release any old lock */ + if (tdb_unlock_record(tdb, tdb->travlocks.off) != 0) + return tdb_null; + tdb->travlocks.off = tdb->travlocks.hash = 0; + + if (tdb_next_lock(tdb, &tdb->travlocks, &rec) <= 0) + return tdb_null; + /* now read the key */ + key.dsize = rec.key_len; + key.dptr =tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec),key.dsize); + if (tdb_unlock(tdb, BUCKET(tdb->travlocks.hash), F_WRLCK) != 0) + TDB_LOG((tdb, 0, "tdb_firstkey: error occurred while tdb_unlocking!\n")); + return key; +} + +/* find the next entry in the database, returning its key */ +TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) +{ + u32 oldhash; + TDB_DATA key = tdb_null; + struct list_struct rec; + unsigned char *k = NULL; + + /* Is locked key the old key? If so, traverse will be reliable. */ + if (tdb->travlocks.off) { + if (tdb_lock(tdb,tdb->travlocks.hash,F_WRLCK)) + return tdb_null; + if (tdb_rec_read(tdb, tdb->travlocks.off, &rec) == -1 + || !(k = tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec), + rec.key_len)) + || memcmp(k, oldkey.dptr, oldkey.dsize) != 0) { + /* No, it wasn't: unlock it and start from scratch */ + if (tdb_unlock_record(tdb, tdb->travlocks.off) != 0) + return tdb_null; + if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) + return tdb_null; + tdb->travlocks.off = 0; + } + + SAFE_FREE(k); + } + + if (!tdb->travlocks.off) { + /* No previous element: do normal find, and lock record */ + tdb->travlocks.off = tdb_find_lock_hash(tdb, oldkey, tdb->hash_fn(&oldkey), F_WRLCK, &rec); + if (!tdb->travlocks.off) + return tdb_null; + tdb->travlocks.hash = BUCKET(rec.full_hash); + if (tdb_lock_record(tdb, tdb->travlocks.off) != 0) { + TDB_LOG((tdb, 0, "tdb_nextkey: lock_record failed (%s)!\n", strerror(errno))); + return tdb_null; + } + } + oldhash = tdb->travlocks.hash; + + /* Grab next record: locks chain and returned record, + unlocks old record */ + if (tdb_next_lock(tdb, &tdb->travlocks, &rec) > 0) { + key.dsize = rec.key_len; + key.dptr = tdb_alloc_read(tdb, tdb->travlocks.off+sizeof(rec), + key.dsize); + /* Unlock the chain of this new record */ + if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) + TDB_LOG((tdb, 0, "tdb_nextkey: WARNING tdb_unlock failed!\n")); + } + /* Unlock the chain of old record */ + if (tdb_unlock(tdb, BUCKET(oldhash), F_WRLCK) != 0) + TDB_LOG((tdb, 0, "tdb_nextkey: WARNING tdb_unlock failed!\n")); + return key; +} + -- cgit From ede8415d61b6791114c65de1c283a4e8c11f1585 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Thu, 22 Sep 2005 03:56:41 +0000 Subject: r10405: added transactions into tdb, and hook them into ldb. See my samba-technical posting for more details on the transactions design. This also adds a number of command line arguments to tdbtorture, making it more flexible, and fixes some lock deadlock conditions in the tdbtorture code. (This used to be commit 06bd8abba942ec9f1e23f5c5d546cbb71ca3a701) --- source4/lib/tdb/common/traverse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index d14be355a2..7d1e99cbe8 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -65,7 +65,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc factor of around 80 in speed on a linux 2.6.x system (testing using ldbtest). */ - tdb_next_hash_chain(tdb, &tlock->hash); + tdb->methods->next_hash_chain(tdb, &tlock->hash); if (tlock->hash == tdb->header.hash_size) { continue; } -- cgit From bd310b792509f7305d7dc029eb4bec109322a4bf Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Thu, 22 Sep 2005 13:12:46 +0000 Subject: r10421: following on discussions with simo, I have worked out a way of allowing searches to proceed while another process is in a transaction, then only upgrading the transaction lock to a write lock on commit. The solution is: - split tdb_traverse() into two calls, called tdb_traverse() and tdb_traverse_read(). The _read() version only gets read locks, and will fail any write operations made in the callback from the traverse. - the normal tdb_traverse() call allows for read or write operations in the callback, but gets the transaction lock, preventing transastions from starting inside the traverse In addition we enforce the following rule that you may not start a transaction within a traverse callback, although you can start a traverse within a transaction With these rules in place I believe all the deadlock possibilities are removed, and we can now allow for searches to happen in parallel with transactions (This used to be commit 7dd31288a701d772e45b1960ac4ce4cc1be782ed) --- source4/lib/tdb/common/traverse.c | 70 ++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 15 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 7d1e99cbe8..335dce4152 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -71,7 +71,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc } } - if (tdb_lock(tdb, tlock->hash, F_WRLCK) == -1) + if (tdb_lock(tdb, tlock->hash, tlock->lock_rw) == -1) return -1; /* No previous record? Start at top of chain. */ @@ -118,7 +118,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc tdb_do_delete(tdb, current, rec) != 0) goto fail; } - tdb_unlock(tdb, tlock->hash, F_WRLCK); + tdb_unlock(tdb, tlock->hash, tlock->lock_rw); want_next = 0; } /* We finished iteration without finding anything */ @@ -126,7 +126,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc fail: tlock->off = 0; - if (tdb_unlock(tdb, tlock->hash, F_WRLCK) != 0) + if (tdb_unlock(tdb, tlock->hash, tlock->lock_rw) != 0) TDB_LOG((tdb, 0, "tdb_next_lock: On error unlock failed!\n")); return -1; } @@ -136,32 +136,33 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc if fn is NULL then it is not called a non-zero return value from fn() indicates that the traversal should stop */ -int tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *private) +static int tdb_traverse_internal(struct tdb_context *tdb, + tdb_traverse_func fn, void *private, + struct tdb_traverse_lock *tl) { TDB_DATA key, dbuf; struct list_struct rec; - struct tdb_traverse_lock tl = { NULL, 0, 0 }; int ret, count = 0; /* This was in the initializaton, above, but the IRIX compiler * did not like it. crh */ - tl.next = tdb->travlocks.next; + tl->next = tdb->travlocks.next; /* fcntl locks don't stack: beware traverse inside traverse */ - tdb->travlocks.next = &tl; + tdb->travlocks.next = tl; /* tdb_next_lock places locks on the record returned, and its chain */ - while ((ret = tdb_next_lock(tdb, &tl, &rec)) > 0) { + while ((ret = tdb_next_lock(tdb, tl, &rec)) > 0) { count++; /* now read the full record */ - key.dptr = tdb_alloc_read(tdb, tl.off + sizeof(rec), + key.dptr = tdb_alloc_read(tdb, tl->off + sizeof(rec), rec.key_len + rec.data_len); if (!key.dptr) { ret = -1; - if (tdb_unlock(tdb, tl.hash, F_WRLCK) != 0) + if (tdb_unlock(tdb, tl->hash, tl->lock_rw) != 0) goto out; - if (tdb_unlock_record(tdb, tl.off) != 0) + if (tdb_unlock_record(tdb, tl->off) != 0) TDB_LOG((tdb, 0, "tdb_traverse: key.dptr == NULL and unlock_record failed!\n")); goto out; } @@ -170,31 +171,70 @@ int tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *private) dbuf.dsize = rec.data_len; /* Drop chain lock, call out */ - if (tdb_unlock(tdb, tl.hash, F_WRLCK) != 0) { + if (tdb_unlock(tdb, tl->hash, tl->lock_rw) != 0) { ret = -1; goto out; } if (fn && fn(tdb, key, dbuf, private)) { /* They want us to terminate traversal */ ret = count; - if (tdb_unlock_record(tdb, tl.off) != 0) { + if (tdb_unlock_record(tdb, tl->off) != 0) { TDB_LOG((tdb, 0, "tdb_traverse: unlock_record failed!\n"));; ret = -1; } - tdb->travlocks.next = tl.next; + tdb->travlocks.next = tl->next; SAFE_FREE(key.dptr); return count; } SAFE_FREE(key.dptr); } out: - tdb->travlocks.next = tl.next; + tdb->travlocks.next = tl->next; if (ret < 0) return -1; else return count; } + +/* + a write style traverse - temporarily marks the db read only +*/ +int tdb_traverse_read(struct tdb_context *tdb, + tdb_traverse_func fn, void *private) +{ + struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; + int ret, read_only = tdb->read_only; + tdb->read_only = 1; + ret = tdb_traverse_internal(tdb, fn, private, &tl); + tdb->read_only = read_only; + return ret; +} + +/* + a write style traverse - needs to get the transaction lock to + prevent deadlocks +*/ +int tdb_traverse(struct tdb_context *tdb, + tdb_traverse_func fn, void *private) +{ + struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; + int ret; + + if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0) == -1) { + TDB_LOG((tdb, 0, "tdb_traverse: failed to get transaction lock\n")); + tdb->ecode = TDB_ERR_LOCK; + return -1; + } + + ret = tdb_traverse_internal(tdb, fn, private, &tl); + + tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); + + return ret; +} + + /* find the first entry in the database and return its key */ TDB_DATA tdb_firstkey(struct tdb_context *tdb) { -- cgit From 86df9ca15a4488bb96f1d7c9244d8f9bb12912ba Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Thu, 22 Sep 2005 13:20:28 +0000 Subject: r10424: for caller convenience, automatically turn a tdb_traverse() into a tdb_traverse_read() for read only databases (This used to be commit 9b53e04377d2ff652c4a9496798d2e3aa0dccab3) --- source4/lib/tdb/common/traverse.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 335dce4152..f8315f5770 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -220,6 +220,10 @@ int tdb_traverse(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; int ret; + + if (tdb->read_only) { + return tdb_traverse_read(tdb, fn, private); + } if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0) == -1) { TDB_LOG((tdb, 0, "tdb_traverse: failed to get transaction lock\n")); -- cgit From 5860aef9cd53da572bef1b86a62a3a5e86da84b0 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 24 Sep 2005 03:43:02 +0000 Subject: r10465: separate out a read_only db from a read-only traversal to ensure we don't end up doing a mmap read only (This used to be commit 294ccfd46a0c4e1af9365d028acdabec03c41ad3) --- source4/lib/tdb/common/traverse.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index f8315f5770..9271b75aa8 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -114,7 +114,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc /* Try to clean dead ones from old traverses */ current = tlock->off; tlock->off = rec->next; - if (!tdb->read_only && + if (!(tdb->read_only || tdb->traverse_read) && tdb_do_delete(tdb, current, rec) != 0) goto fail; } @@ -204,10 +204,10 @@ int tdb_traverse_read(struct tdb_context *tdb, tdb_traverse_func fn, void *private) { struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; - int ret, read_only = tdb->read_only; - tdb->read_only = 1; + int ret; + tdb->traverse_read++; ret = tdb_traverse_internal(tdb, fn, private, &tl); - tdb->read_only = read_only; + tdb->traverse_read--; return ret; } @@ -221,7 +221,7 @@ int tdb_traverse(struct tdb_context *tdb, struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; int ret; - if (tdb->read_only) { + if (tdb->read_only || tdb->traverse_read) { return tdb_traverse_read(tdb, fn, private); } -- cgit From 251aaafe3a9213118ac3a92def9ab2104c40d12a Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 27 Sep 2005 01:26:34 +0000 Subject: r10522: finally got the locking working on solaris10. This adds a read lock on the transaction lock in tdb_traverse_read(). This prevents a pattern of locks which triggers the deadlock detection code in solaris10. I suspect solaris10 is trying to prevent lock starvation by granting locks in the order they were requested, which makes it much easier to produce deadlocks. (This used to be commit 54203aacd138c30826d54c5d9b6cc8d6e9e270f8) --- source4/lib/tdb/common/traverse.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 9271b75aa8..00fe5be923 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -205,9 +205,21 @@ int tdb_traverse_read(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; int ret; + + /* we need to get a read lock on the transaction lock here to + cope with the lock ordering semantics of solaris10 */ + if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_RDLCK, F_SETLKW, 0) == -1) { + TDB_LOG((tdb, 0, "tdb_traverse_read: failed to get transaction lock\n")); + tdb->ecode = TDB_ERR_LOCK; + return -1; + } + tdb->traverse_read++; ret = tdb_traverse_internal(tdb, fn, private, &tl); tdb->traverse_read--; + + tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); + return ret; } -- cgit From 0efc7293181dc06d79c0edc21677f900597fc760 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 1 Mar 2006 20:06:34 +0000 Subject: r13773: Be consistent in the way you get out and free allocated data (This used to be commit 1113d4caa7bea1a7ffb9a50f42c5672bc1d452b4) --- source4/lib/tdb/common/traverse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 00fe5be923..dc1060c6d0 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -173,6 +173,7 @@ static int tdb_traverse_internal(struct tdb_context *tdb, /* Drop chain lock, call out */ if (tdb_unlock(tdb, tl->hash, tl->lock_rw) != 0) { ret = -1; + SAFE_FREE(key.dptr); goto out; } if (fn && fn(tdb, key, dbuf, private)) { @@ -182,9 +183,8 @@ static int tdb_traverse_internal(struct tdb_context *tdb, TDB_LOG((tdb, 0, "tdb_traverse: unlock_record failed!\n"));; ret = -1; } - tdb->travlocks.next = tl->next; SAFE_FREE(key.dptr); - return count; + goto out; } SAFE_FREE(key.dptr); } -- cgit From 1562ecf221d9f5f591e344d6e5c6f19e802b089e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 8 Mar 2006 07:20:10 +0000 Subject: r14029: Fix resource leak in error codepath. Coverity CID #64. Jeremy. (This used to be commit d2e9d5b34baee90060ee9131b3da903309625a56) --- source4/lib/tdb/common/traverse.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index dc1060c6d0..4792af1122 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -289,10 +289,14 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) rec.key_len)) || memcmp(k, oldkey.dptr, oldkey.dsize) != 0) { /* No, it wasn't: unlock it and start from scratch */ - if (tdb_unlock_record(tdb, tdb->travlocks.off) != 0) + if (tdb_unlock_record(tdb, tdb->travlocks.off) != 0) { + SAFE_FREE(k); return tdb_null; - if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) + } + if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) { + SAFE_FREE(k); return tdb_null; + } tdb->travlocks.off = 0; } -- cgit From de1757044611c4b68b96c17b40d64e006c097af7 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 18 Apr 2006 11:26:43 +0000 Subject: r15120: Minimize the diff between Samba3 and Samba4 tdb: In Samba3 we don't allow C++ keywords. Change "private" -> "private_data". Volker (This used to be commit 047671567173e808fece41485bbeaaeebab7d4a2) --- source4/lib/tdb/common/traverse.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 4792af1122..65f64c777b 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -137,7 +137,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc a non-zero return value from fn() indicates that the traversal should stop */ static int tdb_traverse_internal(struct tdb_context *tdb, - tdb_traverse_func fn, void *private, + tdb_traverse_func fn, void *private_data, struct tdb_traverse_lock *tl) { TDB_DATA key, dbuf; @@ -176,7 +176,7 @@ static int tdb_traverse_internal(struct tdb_context *tdb, SAFE_FREE(key.dptr); goto out; } - if (fn && fn(tdb, key, dbuf, private)) { + if (fn && fn(tdb, key, dbuf, private_data)) { /* They want us to terminate traversal */ ret = count; if (tdb_unlock_record(tdb, tl->off) != 0) { @@ -201,7 +201,7 @@ out: a write style traverse - temporarily marks the db read only */ int tdb_traverse_read(struct tdb_context *tdb, - tdb_traverse_func fn, void *private) + tdb_traverse_func fn, void *private_data) { struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; int ret; @@ -215,7 +215,7 @@ int tdb_traverse_read(struct tdb_context *tdb, } tdb->traverse_read++; - ret = tdb_traverse_internal(tdb, fn, private, &tl); + ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_read--; tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); @@ -228,13 +228,13 @@ int tdb_traverse_read(struct tdb_context *tdb, prevent deadlocks */ int tdb_traverse(struct tdb_context *tdb, - tdb_traverse_func fn, void *private) + tdb_traverse_func fn, void *private_data) { struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; int ret; if (tdb->read_only || tdb->traverse_read) { - return tdb_traverse_read(tdb, fn, private); + return tdb_traverse_read(tdb, fn, private_data); } if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0) == -1) { @@ -243,7 +243,7 @@ int tdb_traverse(struct tdb_context *tdb, return -1; } - ret = tdb_traverse_internal(tdb, fn, private, &tl); + ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); -- cgit From d3fee429aee87e9c05a4a606fbf0b60b16dac782 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 3 Jul 2006 06:40:56 +0000 Subject: r16774: This patch modifies the tdb API to allow the logging function to be used as part of ldb. This allows tdb failures to be passed all the way up to Samba's DEBUG system, which allowed easier debugging. Unfortunately I had to extend the tdb API, as the logging function didn't have a context pointer. I've worked over the 'debug levels' in TDB. Most of them were 0, which didn't seem right, as some were trace-like messages. We didn't see any of these previously, except when accessing TDB directly. Andrew Bartlett (This used to be commit 58898092c1ce043f6d698db5065f372b79109e22) --- source4/lib/tdb/common/traverse.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 65f64c777b..90c92042ad 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -100,7 +100,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc /* Detect infinite loops. From "Shlomi Yaakobovich" . */ if (tlock->off == rec->next) { - TDB_LOG((tdb, 0, "tdb_next_lock: loop detected.\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_next_lock: loop detected.\n")); goto fail; } @@ -127,7 +127,7 @@ static int tdb_next_lock(struct tdb_context *tdb, struct tdb_traverse_lock *tloc fail: tlock->off = 0; if (tdb_unlock(tdb, tlock->hash, tlock->lock_rw) != 0) - TDB_LOG((tdb, 0, "tdb_next_lock: On error unlock failed!\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_next_lock: On error unlock failed!\n")); return -1; } @@ -163,7 +163,7 @@ static int tdb_traverse_internal(struct tdb_context *tdb, if (tdb_unlock(tdb, tl->hash, tl->lock_rw) != 0) goto out; if (tdb_unlock_record(tdb, tl->off) != 0) - TDB_LOG((tdb, 0, "tdb_traverse: key.dptr == NULL and unlock_record failed!\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_traverse: key.dptr == NULL and unlock_record failed!\n")); goto out; } key.dsize = rec.key_len; @@ -180,7 +180,7 @@ static int tdb_traverse_internal(struct tdb_context *tdb, /* They want us to terminate traversal */ ret = count; if (tdb_unlock_record(tdb, tl->off) != 0) { - TDB_LOG((tdb, 0, "tdb_traverse: unlock_record failed!\n"));; + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_traverse: unlock_record failed!\n"));; ret = -1; } SAFE_FREE(key.dptr); @@ -209,7 +209,7 @@ int tdb_traverse_read(struct tdb_context *tdb, /* we need to get a read lock on the transaction lock here to cope with the lock ordering semantics of solaris10 */ if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_RDLCK, F_SETLKW, 0) == -1) { - TDB_LOG((tdb, 0, "tdb_traverse_read: failed to get transaction lock\n")); + TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse_read: failed to get transaction lock\n")); tdb->ecode = TDB_ERR_LOCK; return -1; } @@ -238,7 +238,7 @@ int tdb_traverse(struct tdb_context *tdb, } if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0) == -1) { - TDB_LOG((tdb, 0, "tdb_traverse: failed to get transaction lock\n")); + TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse: failed to get transaction lock\n")); tdb->ecode = TDB_ERR_LOCK; return -1; } @@ -268,7 +268,7 @@ TDB_DATA tdb_firstkey(struct tdb_context *tdb) key.dsize = rec.key_len; key.dptr =tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec),key.dsize); if (tdb_unlock(tdb, BUCKET(tdb->travlocks.hash), F_WRLCK) != 0) - TDB_LOG((tdb, 0, "tdb_firstkey: error occurred while tdb_unlocking!\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_firstkey: error occurred while tdb_unlocking!\n")); return key; } @@ -310,7 +310,7 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) return tdb_null; tdb->travlocks.hash = BUCKET(rec.full_hash); if (tdb_lock_record(tdb, tdb->travlocks.off) != 0) { - TDB_LOG((tdb, 0, "tdb_nextkey: lock_record failed (%s)!\n", strerror(errno))); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: lock_record failed (%s)!\n", strerror(errno))); return tdb_null; } } @@ -324,11 +324,11 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) key.dsize); /* Unlock the chain of this new record */ if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) - TDB_LOG((tdb, 0, "tdb_nextkey: WARNING tdb_unlock failed!\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: WARNING tdb_unlock failed!\n")); } /* Unlock the chain of old record */ if (tdb_unlock(tdb, BUCKET(oldhash), F_WRLCK) != 0) - TDB_LOG((tdb, 0, "tdb_nextkey: WARNING tdb_unlock failed!\n")); + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: WARNING tdb_unlock failed!\n")); return key; } -- cgit From 999015bfbec2b9a066a3f2068ae747e2ee393d56 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 14 Aug 2006 09:52:58 +0000 Subject: r17532: merge from SAMBA_3_0 Revision: 17460 First step at fixing the build breakage with the groupmapping test. On Linux, F_RDLCK is defined to 0, for example NetBSD has it at 1. Still does not work fully though. Still investigating. metze (This used to be commit af08e56442367b5d803f61b8554d85e2fe0ce7e9) --- source4/lib/tdb/common/traverse.c | 1 + 1 file changed, 1 insertion(+) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 90c92042ad..6f6510c38a 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -261,6 +261,7 @@ TDB_DATA tdb_firstkey(struct tdb_context *tdb) if (tdb_unlock_record(tdb, tdb->travlocks.off) != 0) return tdb_null; tdb->travlocks.off = tdb->travlocks.hash = 0; + tdb->travlocks.lock_rw = F_RDLCK; if (tdb_next_lock(tdb, &tdb->travlocks, &rec) <= 0) return tdb_null; -- cgit From cba142f1ae71b03266210e254c251683846d7fd7 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 18 Oct 2006 21:41:59 +0000 Subject: r19401: make tdb_lockall() much more efficient, and add a tdb_lockall_read() call which does a read lock on all chains. These will be used to make ldb searches more efficient (This used to be commit de664ec1f8cf179f1d650563272c0de3f7636e2b) --- source4/lib/tdb/common/traverse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 6f6510c38a..fb2371d403 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -208,7 +208,7 @@ int tdb_traverse_read(struct tdb_context *tdb, /* we need to get a read lock on the transaction lock here to cope with the lock ordering semantics of solaris10 */ - if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_RDLCK, F_SETLKW, 0) == -1) { + if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_RDLCK, F_SETLKW, 0, 1) == -1) { TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse_read: failed to get transaction lock\n")); tdb->ecode = TDB_ERR_LOCK; return -1; @@ -218,7 +218,7 @@ int tdb_traverse_read(struct tdb_context *tdb, ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_read--; - tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); + tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); return ret; } @@ -237,7 +237,7 @@ int tdb_traverse(struct tdb_context *tdb, return tdb_traverse_read(tdb, fn, private_data); } - if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0) == -1) { + if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0, 1) == -1) { TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse: failed to get transaction lock\n")); tdb->ecode = TDB_ERR_LOCK; return -1; @@ -245,7 +245,7 @@ int tdb_traverse(struct tdb_context *tdb, ret = tdb_traverse_internal(tdb, fn, private_data, &tl); - tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0); + tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); return ret; } -- cgit From dd64de3d5beeeca870a94f4fb8bbc4d094bbe989 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 30 May 2007 08:15:49 +0000 Subject: r23238: merged transaction lock changes from ctdb this ensures that having the global lock also implies the transaction lock (This used to be commit 9dbb2633d7781fcc5d15b175ef36bfda5eb199bb) --- source4/lib/tdb/common/traverse.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index fb2371d403..1baf90e0f5 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -205,12 +205,10 @@ int tdb_traverse_read(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; int ret; - + /* we need to get a read lock on the transaction lock here to cope with the lock ordering semantics of solaris10 */ - if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_RDLCK, F_SETLKW, 0, 1) == -1) { - TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse_read: failed to get transaction lock\n")); - tdb->ecode = TDB_ERR_LOCK; + if (tdb_transaction_lock(tdb, F_RDLCK)) { return -1; } @@ -218,7 +216,7 @@ int tdb_traverse_read(struct tdb_context *tdb, ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_read--; - tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); + tdb_transaction_unlock(tdb); return ret; } @@ -237,15 +235,13 @@ int tdb_traverse(struct tdb_context *tdb, return tdb_traverse_read(tdb, fn, private_data); } - if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_WRLCK, F_SETLKW, 0, 1) == -1) { - TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_traverse: failed to get transaction lock\n")); - tdb->ecode = TDB_ERR_LOCK; + if (tdb_transaction_lock(tdb, F_WRLCK)) { return -1; } ret = tdb_traverse_internal(tdb, fn, private_data, &tl); - tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); + tdb_transaction_unlock(tdb); return ret; } -- cgit From 90539985db53540f1f7b8d9250105e4bb9cf8093 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 7 Jun 2007 00:00:45 +0000 Subject: r23370: Traverse in tdb wasn't consistently using the travlocks.lock_rw for lock read/write types, it was sometimes using it (tdb_next_lock) and sometimes explicitly using F_WRLCK instead. Change this to consistently use travlocks.lock_rw only. I'm pretty sure about this fix (else I woudn't be checking this in :-) but tridge and Volker please review. Jeremy. (This used to be commit fa548ad75e945ae4d167baffb87140c90cba268c) --- source4/lib/tdb/common/traverse.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 1baf90e0f5..1580a0dd07 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -259,12 +259,15 @@ TDB_DATA tdb_firstkey(struct tdb_context *tdb) tdb->travlocks.off = tdb->travlocks.hash = 0; tdb->travlocks.lock_rw = F_RDLCK; + /* Grab first record: locks chain and returns record. */ if (tdb_next_lock(tdb, &tdb->travlocks, &rec) <= 0) return tdb_null; /* now read the key */ key.dsize = rec.key_len; key.dptr =tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec),key.dsize); - if (tdb_unlock(tdb, BUCKET(tdb->travlocks.hash), F_WRLCK) != 0) + + /* Unlock the hash chain of the record we just read. */ + if (tdb_unlock(tdb, tdb->travlocks.hash, tdb->travlocks.lock_rw) != 0) TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_firstkey: error occurred while tdb_unlocking!\n")); return key; } @@ -279,7 +282,7 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) /* Is locked key the old key? If so, traverse will be reliable. */ if (tdb->travlocks.off) { - if (tdb_lock(tdb,tdb->travlocks.hash,F_WRLCK)) + if (tdb_lock(tdb,tdb->travlocks.hash,tdb->travlocks.lock_rw)) return tdb_null; if (tdb_rec_read(tdb, tdb->travlocks.off, &rec) == -1 || !(k = tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec), @@ -290,7 +293,7 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) SAFE_FREE(k); return tdb_null; } - if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) { + if (tdb_unlock(tdb, tdb->travlocks.hash, tdb->travlocks.lock_rw) != 0) { SAFE_FREE(k); return tdb_null; } @@ -302,7 +305,7 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) if (!tdb->travlocks.off) { /* No previous element: do normal find, and lock record */ - tdb->travlocks.off = tdb_find_lock_hash(tdb, oldkey, tdb->hash_fn(&oldkey), F_WRLCK, &rec); + tdb->travlocks.off = tdb_find_lock_hash(tdb, oldkey, tdb->hash_fn(&oldkey), tdb->travlocks.lock_rw, &rec); if (!tdb->travlocks.off) return tdb_null; tdb->travlocks.hash = BUCKET(rec.full_hash); @@ -313,19 +316,18 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) } oldhash = tdb->travlocks.hash; - /* Grab next record: locks chain and returned record, + /* Grab next record: locks chain and returns record, unlocks old record */ if (tdb_next_lock(tdb, &tdb->travlocks, &rec) > 0) { key.dsize = rec.key_len; key.dptr = tdb_alloc_read(tdb, tdb->travlocks.off+sizeof(rec), key.dsize); /* Unlock the chain of this new record */ - if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0) + if (tdb_unlock(tdb, tdb->travlocks.hash, tdb->travlocks.lock_rw) != 0) TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: WARNING tdb_unlock failed!\n")); } /* Unlock the chain of old record */ - if (tdb_unlock(tdb, BUCKET(oldhash), F_WRLCK) != 0) + if (tdb_unlock(tdb, BUCKET(oldhash), tdb->travlocks.lock_rw) != 0) TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: WARNING tdb_unlock failed!\n")); return key; } - -- cgit From d7942bd2c5eaa2aa62511b018d04ea8ce9c51450 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 7 Jun 2007 00:14:06 +0000 Subject: r23371: Fix the misleading comment I added - it really *should* say "locks chain and returned record", not "and returns record" Jeremy. (This used to be commit fa880e6cc16024f14d10cdc8120ce67bfd1d2eb6) --- source4/lib/tdb/common/traverse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 1580a0dd07..b2bddec18d 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -259,7 +259,7 @@ TDB_DATA tdb_firstkey(struct tdb_context *tdb) tdb->travlocks.off = tdb->travlocks.hash = 0; tdb->travlocks.lock_rw = F_RDLCK; - /* Grab first record: locks chain and returns record. */ + /* Grab first record: locks chain and returned record. */ if (tdb_next_lock(tdb, &tdb->travlocks, &rec) <= 0) return tdb_null; /* now read the key */ @@ -316,7 +316,7 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) } oldhash = tdb->travlocks.hash; - /* Grab next record: locks chain and returns record, + /* Grab next record: locks chain and returned record, unlocks old record */ if (tdb_next_lock(tdb, &tdb->travlocks, &rec) > 0) { key.dsize = rec.key_len; -- cgit From b8d69a7ea2505b706ff7c74d7c97bc89d82dfa07 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 10 Jul 2007 02:46:15 +0000 Subject: r23795: more v2->v3 conversion (This used to be commit 84b468b2f8f2dffda89593f816e8bc6a8b6d42ac) --- source4/lib/tdb/common/traverse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index b2bddec18d..4949b8d438 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -14,7 +14,7 @@ This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either - version 2 of the License, or (at your option) any later version. + version 3 of the License, or (at your option) any later version. This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of -- cgit From 6c973f4e8ccbcb6c9275f8a54e26abb19df7e15a Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 10 Jul 2007 03:42:26 +0000 Subject: r23798: updated old Temple Place FSF addresses to new URL (This used to be commit 40c0919aaa9c1b14bbaebb95ecce53eb0380fdbb) --- source4/lib/tdb/common/traverse.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 4949b8d438..ef8f881084 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -22,8 +22,7 @@ Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public - License along with this library; if not, write to the Free Software - Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + License along with this library; if not, see . */ #include "tdb_private.h" -- cgit From f3e13632813c543583fe1d04203825743aa99111 Mon Sep 17 00:00:00 2001 From: Jelmer Vernooij Date: Sat, 11 Aug 2007 21:19:24 +0000 Subject: r24336: Use standard data type uint32_t rather than tdb-specific u32. (This used to be commit f90a698387c53508862eb6359bd4d1fba1d2b4b0) --- source4/lib/tdb/common/traverse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index ef8f881084..6fc576a55a 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -274,7 +274,7 @@ TDB_DATA tdb_firstkey(struct tdb_context *tdb) /* find the next entry in the database, returning its key */ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) { - u32 oldhash; + uint32_t oldhash; TDB_DATA key = tdb_null; struct list_struct rec; unsigned char *k = NULL; -- cgit From 9170998427ebbb7abfd9b482fb6e0d051bca5205 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 15 Jan 2008 14:05:47 +1100 Subject: merged tdb from ctdb bzr tree (This used to be commit ed0c3a0f74c305b3b8554b05c3f97cf79db8296a) --- source4/lib/tdb/common/traverse.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 6fc576a55a..2bde1270a0 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -238,7 +238,9 @@ int tdb_traverse(struct tdb_context *tdb, return -1; } + tdb->traverse_write++; ret = tdb_traverse_internal(tdb, fn, private_data, &tl); + tdb->traverse_write--; tdb_transaction_unlock(tdb); @@ -330,3 +332,4 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA oldkey) TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_nextkey: WARNING tdb_unlock failed!\n")); return key; } + -- cgit From 61a015a786c52008f4471e62750ad93507bce518 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 18 Jan 2008 15:45:22 +1100 Subject: merged changes from v3-2-test (This used to be commit 7077df3e2e3f171532f6a5ac87d45201736c9c11) --- source4/lib/tdb/common/traverse.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 2bde1270a0..07b0c23858 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -223,6 +223,9 @@ int tdb_traverse_read(struct tdb_context *tdb, /* a write style traverse - needs to get the transaction lock to prevent deadlocks + + WARNING: The data buffer given to the callback fn does NOT meet the + alignment restrictions malloc gives you. */ int tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *private_data) -- cgit From aa7e4b8e9cafaa5139b5111aa8ca042e6b6f65f4 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 20 May 2008 21:54:36 +0200 Subject: Fix nesting tdb_traverse in a transaction Calling tdb_traverse inside a transaction led to the transaction lock being held indefinitely. This was caused by the tdb_transaction_lock/unlock inside tdb_traverse: The transaction code holds the global lock at offset TRANSACTION_LOCK. The call to tdb_transaction_lock does nothing because the transaction_lock is already being held. tdb_transaction_unlock inside tdb_wrap resets tdb->have_transaction_lock but does not release the kernel-level fcntl lock. transaction_commit later on does not release that fcntl lock either, because tdb->have_transaction_lock was already reset by tdb_transaction(). This patch does fix that problem for me. An alternative would be to make tdb->have_transaction_lock a counter that can cope with proper nesting, maybe in other places as well. Volker (This used to be commit 89543005fe2e4934b3c560c937d49304a32a7fc2) --- source4/lib/tdb/common/traverse.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 07b0c23858..5a31742e7b 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -232,20 +232,25 @@ int tdb_traverse(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; int ret; + int in_transaction = (tdb->transaction != NULL); if (tdb->read_only || tdb->traverse_read) { return tdb_traverse_read(tdb, fn, private_data); } - if (tdb_transaction_lock(tdb, F_WRLCK)) { - return -1; + if (!in_transaction) { + if (tdb_transaction_lock(tdb, F_WRLCK)) { + return -1; + } } tdb->traverse_write++; ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_write--; - tdb_transaction_unlock(tdb); + if (!in_transaction) { + tdb_transaction_unlock(tdb); + } return ret; } -- cgit From 6d4424cd45333c3029b0272528485dd2b3f8e620 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 20 May 2008 14:18:58 -0700 Subject: Convert in_transaction to a bool. Add the same fix Volker used for tdb_traverse() to tdb_traverse_read(). Jeremy. (This used to be commit e05ec3047c4fe0cc2e09a812830fc835dc35abea) --- source4/lib/tdb/common/traverse.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'source4/lib/tdb/common/traverse.c') diff --git a/source4/lib/tdb/common/traverse.c b/source4/lib/tdb/common/traverse.c index 5a31742e7b..69c81e6e98 100644 --- a/source4/lib/tdb/common/traverse.c +++ b/source4/lib/tdb/common/traverse.c @@ -204,18 +204,23 @@ int tdb_traverse_read(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; int ret; + bool in_transaction = (tdb->transaction != NULL); /* we need to get a read lock on the transaction lock here to cope with the lock ordering semantics of solaris10 */ - if (tdb_transaction_lock(tdb, F_RDLCK)) { - return -1; + if (!in_transaction) { + if (tdb_transaction_lock(tdb, F_RDLCK)) { + return -1; + } } tdb->traverse_read++; ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_read--; - tdb_transaction_unlock(tdb); + if (!in_transaction) { + tdb_transaction_unlock(tdb); + } return ret; } @@ -232,7 +237,7 @@ int tdb_traverse(struct tdb_context *tdb, { struct tdb_traverse_lock tl = { NULL, 0, 0, F_WRLCK }; int ret; - int in_transaction = (tdb->transaction != NULL); + bool in_transaction = (tdb->transaction != NULL); if (tdb->read_only || tdb->traverse_read) { return tdb_traverse_read(tdb, fn, private_data); -- cgit