Another skiplist bug!

Bron Gondwana brong at fastmail.fm
Mon Jul 27 11:04:26 EDT 2009


Believe it or not, I'm actually not perfect, and there's a doozy
of a bug in skiplists that I'm probably entirely responsible for.

If your foreach callback function returns a non-zero result, then
it double-unlocks.  Wow, what a pain.

Attached patch fixes it.  I've run it on my testbed happily, and
will roll it out to production tomorrow.

Bron ( releases by embarassment - I think it's getting on time for
       a 2.3.15 if we can round up all our bugfixes )
-------------- next part --------------
diff --git a/lib/cyrusdb_skiplist.c b/lib/cyrusdb_skiplist.c
index 36110ca..5b481d4 100644
--- a/lib/cyrusdb_skiplist.c
+++ b/lib/cyrusdb_skiplist.c
@@ -1070,6 +1070,7 @@ int myforeach(struct db *db,
     size_t savebuflen = 0;
     size_t savebufsize;
     int r = 0, cb_r = 0;
+    int need_unlock = 0;
 
     assert(db != NULL);
     assert(prefixlen >= 0);
@@ -1093,6 +1094,7 @@ int myforeach(struct db *db,
 	if ((r = read_lock(db)) < 0) {
 	    return r;
 	}
+	need_unlock = 1;
     } 
 
     ptr = find_node(db, prefix, prefixlen, 0);
@@ -1112,6 +1114,7 @@ int myforeach(struct db *db,
 		if ((r = unlock(db)) < 0) {
 		    return r;
 		}
+		need_unlock = 0;
 	    }
 
 	    /* save KEY, KEYLEN */
@@ -1131,6 +1134,7 @@ int myforeach(struct db *db,
 		if ((r = read_lock(db)) < 0) {
 		    return r;
 		}
+		need_unlock = 1;
 	    } else {
 		/* make sure we're up to date */
 		update_lock(db, *tidptr);
@@ -1161,7 +1165,7 @@ int myforeach(struct db *db,
 	}
     }
 
-    if (!tidptr) {
+    if (need_unlock) {
 	/* release read lock */
 	if ((r = unlock(db)) < 0) {
 	    return r;


More information about the Cyrus-devel mailing list