changeset 2302:ace43b90b388

Tiny concurrency bugfix (race condition on cache->timeNow).
author Ziv Scully <ziv@mit.edu>
date Fri, 20 Nov 2015 10:51:43 -0500
parents 8d772fbf59c1
children 42079884e34a
files src/c/urweb.c
diffstat 1 files changed, 6 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/src/c/urweb.c	Fri Nov 20 03:26:21 2015 -0500
+++ b/src/c/urweb.c	Fri Nov 20 10:51:43 2015 -0500
@@ -4617,7 +4617,8 @@
 }
 
 static unsigned long uw_Sqlcache_getTimeNow(uw_Sqlcache_Cache *cache) {
-  return ++cache->timeNow;
+  // TODO: verify that this makes time comparisons do the Right Thing.
+  return cache->timeNow++;
 }
 
 static unsigned long uw_Sqlcache_timeMax(unsigned long x, unsigned long y) {
@@ -4689,7 +4690,7 @@
   // Returning outside the lock is safe because updates happen at commit time.
   // Those are the only times the returned value or its strings can get freed.
   // Handler output is a new string, so it's safe to free this at commit time.
-  return value && value->timeValid > timeInvalid ? value : NULL;
+  return value && timeInvalid < value->timeValid ? value : NULL;
 }
 
 static void uw_Sqlcache_storeCommitOne(uw_Sqlcache_Cache *cache, char **keys, uw_Sqlcache_Value *value) {
@@ -4712,7 +4713,7 @@
     while (numKeys-- > 0) {
       buf = uw_Sqlcache_keyCopy(buf, keys[numKeys]);
       size_t len = buf - key;
-      
+
       entry = uw_Sqlcache_find(cache, key, len, 1);
       if (!entry) {
         entry = calloc(1, sizeof(uw_Sqlcache_Entry));
@@ -4813,7 +4814,8 @@
   update->keys = uw_Sqlcache_copyKeys(keys, cache->numKeys);
   update->value = value;
   update->next = NULL;
-  value->timeValid = uw_Sqlcache_getTimeNow(cache);
+  // Can't use [uw_Sqlcache_getTimeNow] because it modifies state and we don't have the lock.
+  value->timeValid = cache->timeNow;
   if (ctx->cacheUpdateTail) {
     ctx->cacheUpdateTail->next = update;
   } else {