New test: reveals race (found by Volker) when open occurs during transaction commit.
authorRusty Russell <rusty@rustcorp.com.au>
Tue, 2 Feb 2010 01:22:19 +0000 (11:52 +1030)
committerRusty Russell <rusty@rustcorp.com.au>
Tue, 2 Feb 2010 01:22:19 +0000 (11:52 +1030)
ccan/tdb/test/external-transaction.c
ccan/tdb/test/external-transaction.h
ccan/tdb/test/run-nested-traverse.c
ccan/tdb/test/run-open-during-transaction.c [new file with mode: 0644]
ccan/tdb/test/run-traverse-in-transaction.c

index c03dbc49f6320afd386a5e9f89a2bad936034cf8..1211da9e76d5c1d54702f3725fc34e30eb8d9658 100644 (file)
@@ -7,7 +7,9 @@
 #include <stdlib.h>
 #include <limits.h>
 #include <string.h>
 #include <stdlib.h>
 #include <limits.h>
 #include <string.h>
+#include <errno.h>
 #include <ccan/tdb/tdb.h>
 #include <ccan/tdb/tdb.h>
+#include <ccan/tap/tap.h>
 
 static volatile sig_atomic_t alarmed;
 static void do_alarm(int signum)
 
 static volatile sig_atomic_t alarmed;
 static void do_alarm(int signum)
@@ -15,15 +17,22 @@ static void do_alarm(int signum)
        alarmed++;
 }
 
        alarmed++;
 }
 
-static int do_transaction(const char *name)
+static int do_operation(enum operation op, const char *name)
 {
        TDB_DATA k = { .dptr = (void *)"a", .dsize = 1 };
        TDB_DATA d = { .dptr = (void *)"b", .dsize = 1 };
 {
        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);
+       struct tdb_context *tdb;
 
 
+       tdb = tdb_open(name, 0, op == OPEN_WITH_CLEAR_IF_FIRST ?
+                      TDB_CLEAR_IF_FIRST : TDB_DEFAULT, O_RDWR, 0);
        if (!tdb)
                return -1;
 
        if (!tdb)
                return -1;
 
+       if (op == OPEN || op == OPEN_WITH_CLEAR_IF_FIRST) {
+               tdb_close(tdb);
+               return 0;
+       }
+
        alarmed = 0;
        tdb_setalarm_sigptr(tdb, &alarmed);
 
        alarmed = 0;
        tdb_setalarm_sigptr(tdb, &alarmed);
 
@@ -61,7 +70,7 @@ struct agent *prepare_external_agent(void)
        int pid;
        int command[2], response[2];
        struct sigaction act = { .sa_handler = do_alarm };
        int pid;
        int command[2], response[2];
        struct sigaction act = { .sa_handler = do_alarm };
-       char name[PATH_MAX];
+       char name[1+PATH_MAX];
 
        if (pipe(command) != 0 || pipe(response) != 0)
                return NULL;
 
        if (pipe(command) != 0 || pipe(response) != 0)
                return NULL;
@@ -85,7 +94,9 @@ struct agent *prepare_external_agent(void)
        sigaction(SIGALRM, &act, NULL);
 
        while (read(command[0], name, sizeof(name)) != 0) {
        sigaction(SIGALRM, &act, NULL);
 
        while (read(command[0], name, sizeof(name)) != 0) {
-               int result = do_transaction(name);
+               int result;
+
+               result = do_operation(name[0], name+1);
                if (write(response[1], &result, sizeof(result))
                    != sizeof(result))
                        err(1, "Writing response");
                if (write(response[1], &result, sizeof(result))
                    != sizeof(result))
                        err(1, "Writing response");
@@ -93,13 +104,17 @@ struct agent *prepare_external_agent(void)
        exit(0);
 }
 
        exit(0);
 }
 
-/* Ask the external agent to try to do a transaction. */
-bool external_agent_transaction(struct agent *agent, const char *tdbname)
+/* Ask the external agent to try to do an operation. */
+bool external_agent_operation(struct agent *agent,
+                             enum operation op, const char *tdbname)
 {
        int res;
 {
        int res;
+       char string[1 + strlen(tdbname) + 1];
 
 
-       if (write(agent->cmdfd, tdbname, strlen(tdbname)+1)
-           != strlen(tdbname)+1)
+       string[0] = op;
+       strcpy(string+1, tdbname);
+
+       if (write(agent->cmdfd, string, sizeof(string)) != sizeof(string))
                err(1, "Writing to agent");
 
        if (read(agent->responsefd, &res, sizeof(res)) != sizeof(res))
                err(1, "Writing to agent");
 
        if (read(agent->responsefd, &res, sizeof(res)) != sizeof(res))
@@ -110,3 +125,41 @@ bool external_agent_transaction(struct agent *agent, const char *tdbname)
 
        return res;
 }
 
        return res;
 }
+
+void external_agent_operation_start(struct agent *agent,
+                                   enum operation op, const char *tdbname)
+{
+       char string[1 + strlen(tdbname) + 1];
+
+       string[0] = op;
+       strcpy(string+1, tdbname);
+
+       if (write(agent->cmdfd, string, sizeof(string)) != sizeof(string))
+               err(1, "Writing to agent");
+}
+
+bool external_agent_operation_check(struct agent *agent, bool block, int *res)
+{
+       int flags = fcntl(agent->responsefd, F_GETFL);
+
+       if (block)
+               fcntl(agent->responsefd, F_SETFL, flags & ~O_NONBLOCK);
+       else
+               fcntl(agent->responsefd, F_SETFL, flags | O_NONBLOCK);
+
+       switch (read(agent->responsefd, res, sizeof(*res))) {
+       case sizeof(*res):
+               break;
+       case 0:
+               errx(1, "Agent died?");
+       default:
+               if (!block && (errno == EAGAIN || errno == EWOULDBLOCK))
+                       return false;
+               err(1, "%slocking reading from agent", block ? "B" : "Non-b");
+       }
+
+       if (*res > 1)
+               errx(1, "Agent returned %u\n", *res);
+
+       return true;
+}
index 62a4803bc0ebcd4dd905de608bfe3c9b0549a48a..843b697917522f13690352cbe03798fb253a273e 100644 (file)
@@ -2,10 +2,24 @@
 #define TDB_TEST_EXTERNAL_TRANSACTION_H
 #include <stdbool.h>
 
 #define TDB_TEST_EXTERNAL_TRANSACTION_H
 #include <stdbool.h>
 
+enum operation {
+       OPEN,
+       OPEN_WITH_CLEAR_IF_FIRST,
+       TRANSACTION,
+};
+
 /* Do this before doing any tdb stuff.  Return handle, or -1. */
 struct agent *prepare_external_agent(void);
 
 /* Do this before doing any tdb stuff.  Return handle, or -1. */
 struct agent *prepare_external_agent(void);
 
-/* Ask the external agent to try to do a transaction. */
-bool external_agent_transaction(struct agent *handle, const char *tdbname);
+/* Ask the external agent to try to do an operation. */
+bool external_agent_operation(struct agent *handle,
+                             enum operation op,
+                             const char *tdbname);
+
+/* Ask... */
+void external_agent_operation_start(struct agent *agent,
+                                   enum operation op, const char *tdbname);
 
 
+/* See if they've done it. */
+bool external_agent_operation_check(struct agent *agent, bool block, int *res);
 #endif /* TDB_TEST_EXTERNAL_TRANSACTION_H */
 #endif /* TDB_TEST_EXTERNAL_TRANSACTION_H */
index e78a14efddc49ca9bbbc05f687e47774c84beae4..4470610a37c342b876f119e7ec4b56910f2a3bf0 100644 (file)
@@ -41,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(correct_key(key));
        ok1(correct_data(data));
-       ok1(!external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(!external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
        tdb_traverse(tdb, traverse2, NULL);
 
        /* That should *not* release the transaction lock! */
        tdb_traverse(tdb, traverse2, NULL);
 
        /* That should *not* release the transaction lock! */
-       ok1(!external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(!external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
        return 0;
 }
 
        return 0;
 }
 
@@ -63,7 +63,7 @@ int main(int argc, char *argv[])
                       O_CREAT|O_TRUNC|O_RDWR, 0600);
        ok1(tdb);
 
                       O_CREAT|O_TRUNC|O_RDWR, 0600);
        ok1(tdb);
 
-       ok1(external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
 
        key.dsize = strlen("hi");
        key.dptr = (void *)"hi";
 
        key.dsize = strlen("hi");
        key.dptr = (void *)"hi";
diff --git a/ccan/tdb/test/run-open-during-transaction.c b/ccan/tdb/test/run-open-during-transaction.c
new file mode 100644 (file)
index 0000000..b0eed67
--- /dev/null
@@ -0,0 +1,239 @@
+#define _XOPEN_SOURCE 500
+#include <unistd.h>
+static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset);
+static ssize_t write_check(int fd, const void *buf, size_t count);
+static int fcntl_check(int fd, int cmd, ... /* arg */ );
+static int ftruncate_check(int fd, off_t length);
+
+#define pwrite pwrite_check
+#define write write_check
+#define fcntl fcntl_check
+#define ftruncate ftruncate_check
+
+#include <ccan/tdb/tdb.h>
+#include <ccan/tdb/io.c>
+#include <ccan/tdb/tdb.c>
+#include <ccan/tdb/lock.c>
+#include <ccan/tdb/freelist.c>
+#include <ccan/tdb/traverse.c>
+#include <ccan/tdb/transaction.c>
+#include <ccan/tdb/error.c>
+#include <ccan/tdb/open.c>
+#include <ccan/tap/tap.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <err.h>
+#include "external-transaction.h"
+
+static struct agent *agent;
+static bool agent_pending;
+static bool in_transaction;
+static int errors = 0;
+static bool snapshot_uptodate;
+static char *snapshot;
+static off_t snapshot_len;
+static bool clear_if_first;
+#define TEST_DBNAME "/tmp/test7.tdb"
+
+#undef write
+#undef pwrite
+#undef fcntl
+#undef ftruncate
+
+static void save_file_contents(int fd)
+{
+       struct stat st;
+       int res;
+
+       /* Save copy of file. */
+       stat(TEST_DBNAME, &st);
+       if (snapshot_len != st.st_size) {
+               snapshot = realloc(snapshot, st.st_size * 2);
+               snapshot_len = st.st_size;
+       }
+       res = pread(fd, snapshot, snapshot_len, 0);
+       if (res != snapshot_len)
+               err(1, "Reading %zu bytes = %u", (size_t)snapshot_len, res);
+       snapshot_uptodate = true;
+}
+
+static void check_for_agent(int fd, bool block)
+{
+       struct stat st;
+       int res;
+
+       if (!external_agent_operation_check(agent, block, &res))
+               return;
+
+       if (res != 0)
+               err(1, "Agent failed open");
+       agent_pending = false;
+
+       if (!snapshot_uptodate)
+               return;
+
+       stat(TEST_DBNAME, &st);
+       if (st.st_size != snapshot_len) {
+               diag("Other open changed size from %zu -> %zu",
+                    (size_t)snapshot_len, (size_t)st.st_size);
+               errors++;
+               return;
+       }
+
+       if (pread(fd, snapshot+snapshot_len, snapshot_len, 0) != snapshot_len)
+               err(1, "Reading %zu bytes", (size_t)snapshot_len);
+       if (memcmp(snapshot, snapshot+snapshot_len, snapshot_len) != 0) {
+               diag("File changed");
+               errors++;
+               return;
+       }
+}
+
+static void check_file_contents(int fd)
+{
+       if (agent_pending)
+               check_for_agent(fd, false);
+
+       if (!agent_pending) {
+               save_file_contents(fd);
+
+               /* Ask agent to open file. */
+               external_agent_operation_start(agent,
+                                              clear_if_first ?
+                                              OPEN_WITH_CLEAR_IF_FIRST :
+                                              OPEN,
+                                              TEST_DBNAME);
+               agent_pending = true;
+               /* Hack: give it a chance to run. */
+               sleep(0);
+       }
+
+       check_for_agent(fd, false);
+}
+
+static ssize_t pwrite_check(int fd,
+                           const void *buf, size_t count, off_t offset)
+{
+       ssize_t ret;
+
+       if (in_transaction)
+               check_file_contents(fd);
+
+       snapshot_uptodate = false;
+       ret = pwrite(fd, buf, count, offset);
+       if (ret != count)
+               return ret;
+
+       if (in_transaction)
+               check_file_contents(fd);
+       return ret;
+}
+
+static ssize_t write_check(int fd, const void *buf, size_t count)
+{
+       ssize_t ret;
+
+       if (in_transaction)
+               check_file_contents(fd);
+
+       snapshot_uptodate = false;
+
+       ret = write(fd, buf, count);
+       if (ret != count)
+               return ret;
+
+       if (in_transaction)
+               check_file_contents(fd);
+       return ret;
+}
+
+/* This seems to be a macro for glibc... */
+extern int fcntl(int fd, int cmd, ... /* arg */ );
+
+static int fcntl_check(int fd, int cmd, ... /* arg */ )
+{
+       va_list ap;
+       int ret, arg3;
+       struct flock *fl;
+
+       if (cmd != F_SETLK && cmd != F_SETLKW) {
+               /* This may be totally bogus, but we don't know in general. */
+               va_start(ap, cmd);
+               arg3 = va_arg(ap, int);
+               va_end(ap);
+
+               return fcntl(fd, cmd, arg3);
+       }
+
+       va_start(ap, cmd);
+       fl = va_arg(ap, struct flock *);
+       va_end(ap);
+
+       ret = fcntl(fd, cmd, fl);
+
+       if (in_transaction && fl->l_type == F_UNLCK)
+               check_file_contents(fd);
+       return ret;
+}
+
+static int ftruncate_check(int fd, off_t length)
+{
+       int ret;
+
+       if (in_transaction)
+               check_file_contents(fd);
+
+       snapshot_uptodate = false;
+
+       ret = ftruncate(fd, length);
+
+       if (in_transaction)
+               check_file_contents(fd);
+       return ret;
+}
+
+int main(int argc, char *argv[])
+{
+       const int flags[] = { TDB_DEFAULT,
+                             TDB_CLEAR_IF_FIRST,
+                             TDB_NOMMAP, 
+                             TDB_CLEAR_IF_FIRST | TDB_NOMMAP };
+       int i;
+       struct tdb_context *tdb;
+       TDB_DATA key, data;
+
+       plan_tests(20);
+       agent = prepare_external_agent();
+       if (!agent)
+               err(1, "preparing agent");
+
+       /* Nice ourselves down: we can't tell the difference between agent
+        * blocking on lock, and agent not scheduled. */
+       nice(15);
+
+       for (i = 0; i < sizeof(flags)/sizeof(flags[0]); i++) {
+               unlink(TEST_DBNAME);
+               tdb = tdb_open(TEST_DBNAME, 1024, flags[i],
+                              O_CREAT|O_TRUNC|O_RDWR, 0600);
+               ok1(tdb);
+               clear_if_first = (flags[i] & TDB_CLEAR_IF_FIRST);
+
+               ok1(tdb_transaction_start(tdb) == 0);
+               in_transaction = true;
+               key.dsize = strlen("hi");
+               key.dptr = (void *)"hi";
+               data.dptr = (void *)"world";
+               data.dsize = strlen("world");
+
+               ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
+               ok1(tdb_transaction_commit(tdb) == 0);
+               if (agent_pending)
+                       check_for_agent(tdb->fd, true);
+               ok(errors == 0, "We had %u errors", errors);
+
+               tdb_close(tdb);
+       }
+
+       return exit_status();
+}
index 5b1c870c7f7a04ddeeaa8b67de2874693df93eff..37a4f6d3ffd9ee760fef70de4746fc922f66285b 100644 (file)
@@ -57,21 +57,21 @@ int main(int argc, char *argv[])
 
        ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
 
 
        ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
 
-       ok1(external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
 
        ok1(tdb_transaction_start(tdb) == 0);
 
        ok1(tdb_transaction_start(tdb) == 0);
-       ok1(!external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(!external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
        tdb_traverse(tdb, traverse, NULL);
 
        /* That should *not* release the transaction lock! */
        tdb_traverse(tdb, traverse, NULL);
 
        /* That should *not* release the transaction lock! */
-       ok1(!external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(!external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
        tdb_traverse_read(tdb, traverse, NULL);
 
        /* That should *not* release the transaction lock! */
        tdb_traverse_read(tdb, traverse, NULL);
 
        /* That should *not* release the transaction lock! */
-       ok1(!external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(!external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
        ok1(tdb_transaction_commit(tdb) == 0);
        /* Now we should be fine. */
        ok1(tdb_transaction_commit(tdb) == 0);
        /* Now we should be fine. */
-       ok1(external_agent_transaction(agent, tdb_name(tdb)));
+       ok1(external_agent_operation(agent, TRANSACTION, tdb_name(tdb)));
 
        tdb_close(tdb);
 
 
        tdb_close(tdb);