Changeset: 1611826cd43e for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/1611826cd43e
Modified Files:
        gdk/gdk_bat.c
        gdk/gdk_batop.c
        gdk/gdk_heap.c
        gdk/gdk_private.h
        gdk/gdk_string.c
Branch: default
Log Message:

Move theaplock to outside of HEAPgrow to avoid data races.


diffs (179 lines):

diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -527,6 +527,7 @@ gdk_return
 BATextend(BAT *b, BUN newcap)
 {
        size_t theap_size;
+       gdk_return rc = GDK_SUCCEED;
 
        assert(newcap <= BUN_MAX);
        BATcheck(b, GDK_FAIL);
@@ -549,14 +550,18 @@ BATextend(BAT *b, BUN newcap)
        } else {
                theap_size = (size_t) newcap << b->tshift;
        }
-       b->batCapacity = newcap;
 
+       MT_lock_set(&b->theaplock);
        if (b->theap->base) {
                TRC_DEBUG(HEAP, "HEAPgrow in BATextend %s %zu %zu\n",
                          b->theap->filename, b->theap->size, theap_size);
-               return HEAPgrow(&b->theaplock, &b->theap, theap_size, 
b->batRestricted == BAT_READ);
+               rc = HEAPgrow(&b->theap, theap_size, b->batRestricted == 
BAT_READ);
        }
-       return GDK_SUCCEED;
+
+       b->batCapacity = newcap;
+       MT_lock_unset(&b->theaplock);
+
+       return rc;
 }
 
 
diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c
--- a/gdk/gdk_batop.c
+++ b/gdk/gdk_batop.c
@@ -138,11 +138,14 @@ insert_string_bat(BAT *b, BATiter *ni, s
                                        MT_thread_setalgorithm("append vheap");
                                }
 
-                               if (HEAPgrow(&b->theaplock, &b->tvheap, toff + 
ni->vh->size, force) != GDK_SUCCEED) {
+                               MT_lock_set(&b->theaplock);
+                               if (HEAPgrow(&b->tvheap, toff + ni->vh->size, 
force) != GDK_SUCCEED) {
+                                       MT_lock_unset(&b->theaplock);
                                        return GDK_FAIL;
                                }
                                memcpy(b->tvheap->base + toff, ni->vh->base, 
ni->vhfree);
                                b->tvheap->free = toff + ni->vhfree;
+                               MT_lock_unset(&b->theaplock);
                        }
                }
        }
@@ -418,8 +421,11 @@ append_varsized_bat(BAT *b, BATiter *ni,
        if (BATcount(b) == 0 && BATatoms[b->ttype].atomFix == NULL &&
            ci->tpe == cand_dense && ci->ncand == ni->count) {
                /* just copy the heaps */
-               if (HEAPgrow(&b->theaplock, &b->tvheap, ni->vhfree, false) != 
GDK_SUCCEED)
+               MT_lock_set(&b->theaplock);
+               if (HEAPgrow(&b->tvheap, ni->vhfree, false) != GDK_SUCCEED) {
+                       MT_lock_unset(&b->theaplock);
                        return GDK_FAIL;
+               }
                memcpy(b->theap->base, ni->base, ni->hfree);
                memcpy(b->tvheap->base, ni->vh->base, ni->vhfree);
                b->theap->free = ni->hfree;
@@ -437,6 +443,7 @@ append_varsized_bat(BAT *b, BATiter *ni,
                b->tminpos = ni->minpos;
                b->tmaxpos = ni->maxpos;
                b->tunique_est = ni->unique_est;
+               MT_lock_unset(&b->theaplock);
                return GDK_SUCCEED;
        }
        /* copy data from n to b */
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -86,16 +86,15 @@ decompose_filename(str nme)
        return ext;
 }
 
+/* this function is called with the theaplock held */
 gdk_return
-HEAPgrow(MT_Lock *lock, Heap **hp, size_t size, bool mayshare)
+HEAPgrow(Heap **hp, size_t size, bool mayshare)
 {
        Heap *new;
 
-       MT_lock_set(lock);
        ATOMIC_BASE_TYPE refs = ATOMIC_GET(&(*hp)->refs);
        if ((refs & HEAPREFS) == 1) {
                gdk_return rc = HEAPextend((*hp), size, mayshare);
-               MT_lock_unset(lock);
                return rc;
        }
        new = GDKmalloc(sizeof(Heap));
@@ -126,7 +125,6 @@ HEAPgrow(MT_Lock *lock, Heap **hp, size_
                        new = NULL;
                }
        }
-       MT_lock_unset(lock);
        return new ? GDK_SUCCEED : GDK_FAIL;
 }
 
@@ -1118,11 +1116,14 @@ HEAP_malloc(BAT *b, size_t nbytes)
 
                /* Increase the size of the heap. */
                TRC_DEBUG(HEAP, "HEAPextend in HEAP_malloc %s %zu %zu\n", 
heap->filename, heap->size, newsize);
-               if (HEAPgrow(&b->theaplock, &b->tvheap, newsize, false) != 
GDK_SUCCEED) {
+               MT_lock_set(&b->theaplock);
+               if (HEAPgrow(&b->tvheap, newsize, false) != GDK_SUCCEED) {
+                       MT_lock_unset(&b->theaplock);
                        return (var_t) -1;
                }
                heap = b->tvheap;
                heap->free = newsize;
+               MT_lock_unset(&b->theaplock);
                hheader = HEAP_index(heap, 0, HEADER);
 
                blockp = HEAP_index(heap, block, CHUNK);
diff --git a/gdk/gdk_private.h b/gdk/gdk_private.h
--- a/gdk/gdk_private.h
+++ b/gdk/gdk_private.h
@@ -202,7 +202,7 @@ gdk_return HEAPcopy(Heap *dst, Heap *src
        __attribute__((__visibility__("hidden")));
 void HEAPfree(Heap *h, bool remove)
        __attribute__((__visibility__("hidden")));
-gdk_return HEAPgrow(MT_Lock *lock, Heap **old, size_t size, bool mayshare)
+gdk_return HEAPgrow(Heap **old, size_t size, bool mayshare)
        __attribute__((__visibility__("hidden")));
 gdk_return HEAPload(Heap *h, const char *nme, const char *ext, bool trunc)
        __attribute__((__warn_unused_result__))
diff --git a/gdk/gdk_string.c b/gdk/gdk_string.c
--- a/gdk/gdk_string.c
+++ b/gdk/gdk_string.c
@@ -188,14 +188,17 @@ strPut(BAT *b, var_t *dst, const void *V
        BUN off;
 
        if (h->free == 0) {
+               MT_lock_set(&b->theaplock);
                if (h->size < GDK_STRHASHTABLE * sizeof(stridx_t) + BATTINY * 
GDK_VARALIGN) {
-                       if (HEAPgrow(&b->theaplock, &b->tvheap, 
GDK_STRHASHTABLE * sizeof(stridx_t) + BATTINY * GDK_VARALIGN, true) != 
GDK_SUCCEED) {
+                       if (HEAPgrow(&b->tvheap, GDK_STRHASHTABLE * 
sizeof(stridx_t) + BATTINY * GDK_VARALIGN, true) != GDK_SUCCEED) {
+                               MT_lock_unset(&b->theaplock);
                                return (var_t) -1;
                        }
                        h = b->tvheap;
                }
                h->free = GDK_STRHASHTABLE * sizeof(stridx_t);
                h->dirty = true;
+               MT_lock_unset(&b->theaplock);
 #ifdef NDEBUG
                memset(h->base, 0, h->free);
 #else
@@ -279,10 +282,13 @@ strPut(BAT *b, var_t *dst, const void *V
                        return (var_t) -1;
                }
                TRC_DEBUG(HEAP, "HEAPextend in strPut %s %zu %zu\n", 
h->filename, h->size, newsize);
-               if (HEAPgrow(&b->theaplock, &b->tvheap, newsize, true) != 
GDK_SUCCEED) {
+               MT_lock_set(&b->theaplock);
+               if (HEAPgrow(&b->tvheap, newsize, true) != GDK_SUCCEED) {
+                       MT_lock_unset(&b->theaplock);
                        return (var_t) -1;
                }
                h = b->tvheap;
+               MT_lock_unset(&b->theaplock);
 
                /* make bucket point into the new heap */
                bucket = ((stridx_t *) h->base) + off;
@@ -294,8 +300,10 @@ strPut(BAT *b, var_t *dst, const void *V
        if (pad > 0)
                memset(h->base + h->free, 0, pad);
        memcpy(h->base + pos, v, len);
+       MT_lock_set(&b->theaplock);
        h->free += pad + len;
        h->dirty = true;
+       MT_lock_unset(&b->theaplock);
 
        /* maintain hash table */
        if (GDK_ELIMBASE(pos) == 0) {   /* small string heap: link the next 
pointer */
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org

Reply via email to