From: Michael Vrable <mvrable@cs.ucsd.edu>
Date: Fri, 10 Dec 2010 17:12:50 +0000 (-0800)
Subject: Fix a race condition in bluesky_mmap_unref.
X-Git-Url: https://git.vrable.net/?a=commitdiff_plain;h=0e94ccdf181e154c366dc5169c970b149af000e5;p=bluesky.git

Fix a race condition in bluesky_mmap_unref.

It turned out that the old code had a race--between decrementing and
testing the reference count and acquiring the lock, another thread could
potentially acquire a reference, release the reference, and then unmap the
data.  This still left the reference count as zero, so a second unmap was
attempted.  Now, use mmap->addr to detect if the unmap needs to be done or
not which should eliminate this race.
---

diff --git a/bluesky/log.c b/bluesky/log.c
index 6475b5d..03f4fb3 100644
--- a/bluesky/log.c
+++ b/bluesky/log.c
@@ -482,7 +482,8 @@ static void cloudlog_partial_fetch_start(BlueSkyCacheFile *cachefile,
                                          size_t offset, size_t length)
 {
     g_atomic_int_inc(&cachefile->refcount);
-    g_print("Starting fetch of %s from cloud\n", cachefile->filename);
+    g_print("Starting partial fetch of %s from cloud (%zd + %zd)\n",
+            cachefile->filename, offset, length);
     BlueSkyStoreAsync *async = bluesky_store_async_new(cachefile->fs->store);
     async->op = STORE_OP_GET;
     async->key = g_strdup(cachefile->filename);
@@ -700,7 +701,7 @@ BlueSkyRCStr *bluesky_log_map_object(BlueSkyCloudLog *item, gboolean map_data)
                             file_offset);
                 }
                 if (range_request) {
-                    uint64_t start = 0, length = 0, end;
+                    uint64_t start = file_offset, length = file_size, end;
                     if (map->prefetches != NULL)
                         bluesky_rangeset_get_extents(map->prefetches,
                                                      &start, &length);
@@ -717,7 +718,7 @@ BlueSkyRCStr *bluesky_log_map_object(BlueSkyCloudLog *item, gboolean map_data)
             } else if (rangeitem->start == file_offset
                        && rangeitem->length == file_size) {
                 if (bluesky_verbose)
-                    g_print("Item now available.\n");
+                    g_print("Item %zd now available.\n", file_offset);
                 break;
             }
         }
@@ -748,7 +749,7 @@ void bluesky_mmap_unref(BlueSkyCacheFile *mmap)
 
     if (g_atomic_int_dec_and_test(&mmap->mapcount)) {
         g_mutex_lock(mmap->lock);
-        if (g_atomic_int_get(&mmap->mapcount) == 0) {
+        if (mmap->addr != NULL) {
             if (bluesky_verbose)
                 g_print("Unmapped log segment %d...\n", mmap->log_seq);
             munmap((void *)mmap->addr, mmap->len);
@@ -757,6 +758,7 @@ void bluesky_mmap_unref(BlueSkyCacheFile *mmap)
         }
         g_mutex_unlock(mmap->lock);
     }
+    g_assert(g_atomic_int_get(&mmap->mapcount) >= 0);
 }
 
 /******************************* JOURNAL REPLAY *******************************