From 15758cca5f15f362edaf9da25f831d10e58b1b9a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 21 Jul 2009 16:54:48 +0930 Subject: [PATCH] Neaten tdb to sync with samba version of locking fix. Also, actually test locking, rather than just lock count (bit me in samba fix) --- ccan/tdb/lock.c | 17 +++- ccan/tdb/tdb_private.h | 2 +- ccan/tdb/test/external-transaction.c | 107 ++++++++++++++++++++ ccan/tdb/test/external-transaction.h | 11 ++ ccan/tdb/test/run-nested-traverse.c | 18 +++- ccan/tdb/test/run-traverse-in-transaction.c | 23 +++-- 6 files changed, 160 insertions(+), 18 deletions(-) create mode 100644 ccan/tdb/test/external-transaction.c create mode 100644 ccan/tdb/test/external-transaction.h diff --git a/ccan/tdb/lock.c b/ccan/tdb/lock.c index bcd560d7..326d38e7 100644 --- a/ccan/tdb/lock.c +++ b/ccan/tdb/lock.c @@ -304,17 +304,18 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype) if (tdb->global_lock.count) { return 0; } - if (tdb->have_transaction_lock) { - tdb->have_transaction_lock++; + if (tdb->transaction_lock_count > 0) { + tdb->transaction_lock_count++; return 0; } + if (tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, ltype, F_SETLKW, 0, 1) == -1) { TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_lock: failed to get transaction lock\n")); tdb->ecode = TDB_ERR_LOCK; return -1; } - tdb->have_transaction_lock++; + tdb->transaction_lock_count++; return 0; } @@ -323,13 +324,19 @@ int tdb_transaction_lock(struct tdb_context *tdb, int ltype) */ int tdb_transaction_unlock(struct tdb_context *tdb) { + int ret; if (tdb->global_lock.count) { return 0; } - if (--tdb->have_transaction_lock) { + if (tdb->transaction_lock_count > 1) { + tdb->transaction_lock_count--; return 0; } - return tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); + ret = tdb->methods->tdb_brlock(tdb, TRANSACTION_LOCK, F_UNLCK, F_SETLKW, 0, 1); + if (ret == 0) { + tdb->transaction_lock_count = 0; + } + return ret; } diff --git a/ccan/tdb/tdb_private.h b/ccan/tdb/tdb_private.h index 48364728..ceb4358c 100644 --- a/ccan/tdb/tdb_private.h +++ b/ccan/tdb/tdb_private.h @@ -215,7 +215,7 @@ struct tdb_context { struct tdb_transaction *transaction; int page_size; int max_dead_records; - int have_transaction_lock; + int transaction_lock_count; int tracefd; volatile sig_atomic_t *interrupt_sig_ptr; }; diff --git a/ccan/tdb/test/external-transaction.c b/ccan/tdb/test/external-transaction.c new file mode 100644 index 00000000..c1442c21 --- /dev/null +++ b/ccan/tdb/test/external-transaction.c @@ -0,0 +1,107 @@ +#include "external-transaction.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static volatile sig_atomic_t alarmed; +static void do_alarm(int signum) +{ + alarmed++; +} + +static int do_transaction(const char *name) +{ + TDB_DATA k = { .dptr = (void *)"a", .dsize = 1 }; + TDB_DATA d = { .dptr = (void *)"b", .dsize = 1 }; + struct tdb_context *tdb = tdb_open(name, 0, 0, O_RDWR, 0); + + if (!tdb) + return -1; + + alarmed = 0; + tdb_setalarm_sigptr(tdb, &alarmed); + + alarm(1); + if (tdb_transaction_start(tdb) != 0) + goto maybe_alarmed; + + if (tdb_store(tdb, k, d, 0) != 0) { + tdb_transaction_cancel(tdb); + tdb_close(tdb); + return -2; + } + + if (tdb_transaction_commit(tdb) == 0) { + tdb_delete(tdb, k); + tdb_close(tdb); + return 1; + } + + tdb_delete(tdb, k); +maybe_alarmed: + tdb_close(tdb); + if (alarmed) + return 0; + return -3; +} + + +/* Do this before doing any tdb stuff. Return handle, or -1. */ +int prepare_external_agent(void) +{ + int pid; + int command[2], response[2]; + struct sigaction act = { .sa_handler = do_alarm }; + char name[PATH_MAX]; + + if (pipe(command) != 0 || pipe(response) != 0) + return -1; + + pid = fork(); + if (pid < 0) + return -1; + + if (pid != 0) { + close(command[0]); + close(response[1]); + /* FIXME: Make fds consective. */ + dup2(command[1]+1, response[1]); + return command[1]; + } + + close(command[1]); + close(response[0]); + sigaction(SIGALRM, &act, NULL); + + while (read(command[0], name, sizeof(name)) != 0) { + int result = do_transaction(name); + if (write(response[1], &result, sizeof(result)) + != sizeof(result)) + err(1, "Writing response"); + } + exit(0); +} + +/* Ask the external agent to try to do a transaction. */ +bool external_agent_transaction(int handle, const char *tdbname) +{ + int res; + + if (write(handle, tdbname, strlen(tdbname)+1) + != strlen(tdbname)+1) + err(1, "Writing to agent"); + + if (read(handle+1, &res, sizeof(res)) != sizeof(res)) + err(1, "Reading from agent"); + + if (res > 1) + errx(1, "Agent returned %u\n", res); + + return res; +} diff --git a/ccan/tdb/test/external-transaction.h b/ccan/tdb/test/external-transaction.h new file mode 100644 index 00000000..8f28a624 --- /dev/null +++ b/ccan/tdb/test/external-transaction.h @@ -0,0 +1,11 @@ +#ifndef TDB_TEST_EXTERNAL_TRANSACTION_H +#define TDB_TEST_EXTERNAL_TRANSACTION_H +#include + +/* Do this before doing any tdb stuff. Return handle, or -1. */ +int prepare_external_agent(void); + +/* Ask the external agent to try to do a transaction. */ +bool external_agent_transaction(int handle, const char *tdbname); + +#endif /* TDB_TEST_EXTERNAL_TRANSACTION_H */ diff --git a/ccan/tdb/test/run-nested-traverse.c b/ccan/tdb/test/run-nested-traverse.c index f4a36b62..df4c7da1 100644 --- a/ccan/tdb/test/run-nested-traverse.c +++ b/ccan/tdb/test/run-nested-traverse.c @@ -12,6 +12,9 @@ #include #include #include +#include "external-transaction.h" + +static int agent; static bool correct_key(TDB_DATA key) { @@ -38,11 +41,11 @@ static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, { ok1(correct_key(key)); ok1(correct_data(data)); - ok1(tdb->have_transaction_lock); + ok1(!external_agent_transaction(agent, tdb_name(tdb))); tdb_traverse(tdb, traverse2, NULL); /* That should *not* release the transaction lock! */ - ok1(tdb->have_transaction_lock); + ok1(!external_agent_transaction(agent, tdb_name(tdb))); return 0; } @@ -51,12 +54,17 @@ int main(int argc, char *argv[]) struct tdb_context *tdb; TDB_DATA key, data; - plan_tests(14); - tdb = tdb_open("/tmp/test.tdb", 1024, TDB_CLEAR_IF_FIRST, + plan_tests(15); + agent = prepare_external_agent(); + if (agent < 0) + err(1, "preparing agent"); + + tdb = tdb_open("/tmp/test3.tdb", 1024, TDB_CLEAR_IF_FIRST, O_CREAT|O_TRUNC|O_RDWR, 0600); ok1(tdb); - /* Tickle bug on appending zero length buffer to zero length buffer. */ + ok1(external_agent_transaction(agent, tdb_name(tdb))); + key.dsize = strlen("hi"); key.dptr = (void *)"hi"; data.dptr = (void *)"world"; diff --git a/ccan/tdb/test/run-traverse-in-transaction.c b/ccan/tdb/test/run-traverse-in-transaction.c index b184474f..e8d6819a 100644 --- a/ccan/tdb/test/run-traverse-in-transaction.c +++ b/ccan/tdb/test/run-traverse-in-transaction.c @@ -12,6 +12,9 @@ #include #include #include +#include "external-transaction.h" + +static int agent; static bool correct_key(TDB_DATA key) { @@ -38,12 +41,15 @@ int main(int argc, char *argv[]) struct tdb_context *tdb; TDB_DATA key, data; - plan_tests(12); - tdb = tdb_open("/tmp/test.tdb", 1024, TDB_CLEAR_IF_FIRST, + plan_tests(13); + agent = prepare_external_agent(); + if (agent < 0) + err(1, "preparing agent"); + + tdb = tdb_open("/tmp/test2.tdb", 1024, TDB_CLEAR_IF_FIRST, O_CREAT|O_TRUNC|O_RDWR, 0600); ok1(tdb); - /* Tickle bug on appending zero length buffer to zero length buffer. */ key.dsize = strlen("hi"); key.dptr = (void *)"hi"; data.dptr = (void *)"world"; @@ -51,18 +57,21 @@ int main(int argc, char *argv[]) ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); + ok1(external_agent_transaction(agent, tdb_name(tdb))); + ok1(tdb_transaction_start(tdb) == 0); - ok(tdb->have_transaction_lock, "Transaction lock in transaction"); + ok1(!external_agent_transaction(agent, tdb_name(tdb))); tdb_traverse(tdb, traverse, NULL); /* That should *not* release the transaction lock! */ - ok(tdb->have_transaction_lock, "Transaction lock after traverse"); + ok1(!external_agent_transaction(agent, tdb_name(tdb))); tdb_traverse_read(tdb, traverse, NULL); /* That should *not* release the transaction lock! */ - ok(tdb->have_transaction_lock, "Transaction lock after traverse_read"); + ok1(!external_agent_transaction(agent, tdb_name(tdb))); ok1(tdb_transaction_commit(tdb) == 0); - ok(!tdb->have_transaction_lock, "Transaction unlock"); + /* Now we should be fine. */ + ok1(external_agent_transaction(agent, tdb_name(tdb))); tdb_close(tdb); -- 2.39.2