tdb2: check pid before unlock.
authorRusty Russell <rusty@rustcorp.com.au>
Tue, 10 May 2011 01:37:21 +0000 (11:07 +0930)
committerRusty Russell <rusty@rustcorp.com.au>
Tue, 10 May 2011 01:37:21 +0000 (11:07 +0930)
The original code assumed that unlocking would fail if we didn't have a lock;
this isn't true (at least, on my machine).  So we have to always check the
pid before unlocking.

ccan/tdb2/lock.c
ccan/tdb2/test/run-fork-test.c

index 4033d0afe813e59bf4dee4b8bcc9e5d34757c298..49313a5419cb7b83b7f672aaff32f5942ddf079b 100644 (file)
@@ -37,7 +37,7 @@ static enum TDB_ERROR owner_conflict(struct tdb_context *tdb, const char *call)
                          call);
 }
 
-/* If we fork, we no longer really own locks: preserves errno */
+/* If we fork, we no longer really own locks. */
 static bool check_lock_pid(struct tdb_context *tdb,
                           const char *call, bool log)
 {
@@ -224,16 +224,14 @@ static enum TDB_ERROR tdb_brlock(struct tdb_context *tdb,
 static enum TDB_ERROR tdb_brunlock(struct tdb_context *tdb,
                                   int rw_type, tdb_off_t offset, size_t len)
 {
-       int ret;
-
        if (tdb->flags & TDB_NOLOCK) {
                return TDB_SUCCESS;
        }
 
-       ret = unlock(tdb, rw_type, offset, len);
+       if (!check_lock_pid(tdb, "tdb_brunlock", true))
+               return TDB_ERR_LOCK;
 
-       /* If we fail, *then* we verify that we owned the lock.  If not, ok. */
-       if (ret == -1 && check_lock_pid(tdb, "tdb_brunlock", false)) {
+       if (unlock(tdb, rw_type, offset, len) == -1) {
                return tdb_logerr(tdb, TDB_ERR_LOCK, TDB_LOG_ERROR,
                                  "tdb_brunlock failed (fd=%d) at offset %zu"
                                  " rw_type=%d len=%zu: %s",
@@ -851,6 +849,10 @@ void tdb_lock_cleanup(struct tdb_context *tdb)
 {
        unsigned int i;
 
+       /* We don't want to warn: they're allowed to close tdb after fork. */
+       if (!check_lock_pid(tdb, "tdb_close", false))
+               return;
+
        while (tdb->file->allrecord_lock.count
               && tdb->file->allrecord_lock.owner == tdb) {
                tdb_allrecord_unlock(tdb, tdb->file->allrecord_lock.ltype);
index ff1caa06e0f081f50e7f669feee24f23ea00a2d8..e9813e0a0f01051565a280a825dfa34ef9f50705 100644 (file)
@@ -85,10 +85,10 @@ int main(int argc, char *argv[])
                                return 2;
 
                        tdb_chainunlock(tdb, key);
-                       if (tap_log_messages != 2)
+                       if (tap_log_messages != 3)
                                return 3;
                        tdb_close(tdb);
-                       if (tap_log_messages != 2)
+                       if (tap_log_messages != 3)
                                return 4;
                        return 0;
                }