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