Changeset: d0abd3472eda for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/d0abd3472eda
Modified Files:
gdk/gdk_align.c
gdk/gdk_bat.c
gdk/gdk_batop.c
gdk/gdk_heap.c
gdk/gdk_imprints.c
gdk/gdk_private.h
gdk/gdk_strimps.c
Branch: default
Log Message:
Remove data race: put Heap.remove flag inside of atomic var .refs.
diffs (220 lines):
diff --git a/gdk/gdk_align.c b/gdk/gdk_align.c
--- a/gdk/gdk_align.c
+++ b/gdk/gdk_align.c
@@ -165,6 +165,7 @@ BATmaterialize(BAT *b)
{
BUN cnt;
Heap *tail;
+ Heap *h, *vh = NULL;
BUN p, q;
oid t, *x;
@@ -210,7 +211,7 @@ BATmaterialize(BAT *b)
ATOMIC_INIT(&tail->refs, 1);
/* point of no return */
MT_lock_set(&b->theaplock);
- assert(ATOMIC_GET(&b->theap->refs) > 0);
+ assert((ATOMIC_GET(&b->theap->refs) & HEAPREFS) > 0);
/* can only look at tvheap when lock is held */
if (complex_cand(b)) {
assert(b->batRole == TRANSIENT);
@@ -245,10 +246,10 @@ BATmaterialize(BAT *b)
}
assert(n == q);
}
- HEAPdecref(b->tvheap, true);
+ vh = b->tvheap;
b->tvheap = NULL;
}
- HEAPdecref(b->theap, false);
+ h = b->theap;
b->theap = tail;
b->tbaseoff = 0;
b->theap->dirty = true;
@@ -258,6 +259,9 @@ BATmaterialize(BAT *b)
BATsetdims(b, 0);
b->batDirtydesc = true;
BATsetcount(b, b->batCount);
+ HEAPdecref(h, false);
+ if (vh)
+ HEAPdecref(vh, true);
return GDK_SUCCEED;
}
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -682,12 +682,12 @@ BATfree(BAT *b)
b->tunique_est = (double) nunique;
}
if (b->theap) {
- assert(ATOMIC_GET(&b->theap->refs) == 1);
+ assert((ATOMIC_GET(&b->theap->refs) & HEAPREFS) == 1);
assert(b->theap->parentid == b->batCacheid);
HEAPfree(b->theap, false);
}
if (b->tvheap) {
- assert(ATOMIC_GET(&b->tvheap->refs) == 1);
+ assert((ATOMIC_GET(&b->tvheap->refs) & HEAPREFS) == 1);
assert(b->tvheap->parentid == b->batCacheid);
HEAPfree(b->tvheap, false);
}
diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c
--- a/gdk/gdk_batop.c
+++ b/gdk/gdk_batop.c
@@ -41,9 +41,10 @@ unshare_varsized_heap(BAT *b)
ATOMIC_INIT(&h->refs, 1);
MT_lock_set(&b->theaplock);
int parent = b->tvheap->parentid;
- HEAPdecref(b->tvheap, false);
+ Heap *oh = b->tvheap;
b->tvheap = h;
MT_lock_unset(&b->theaplock);
+ HEAPdecref(oh, false);
BBPunshare(parent);
BBPunfix(parent);
}
@@ -418,11 +419,12 @@ append_varsized_bat(BAT *b, BAT *n, stru
return GDK_FAIL;
}
BBPunshare(b->tvheap->parentid);
+ ATOMIC_INIT(&h->refs, 1);
MT_lock_set(&b->theaplock);
- HEAPdecref(b->tvheap, false);
- ATOMIC_INIT(&h->refs, 1);
+ Heap *oh = b->tvheap;
b->tvheap = h;
MT_lock_unset(&b->theaplock);
+ HEAPdecref(oh, false);
}
/* copy data from n to b */
r = BATcount(b);
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -92,7 +92,8 @@ HEAPgrow(MT_Lock *lock, Heap **hp, size_
Heap *new;
MT_lock_set(lock);
- if (ATOMIC_GET(&(*hp)->refs) == 1) {
+ 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;
@@ -103,13 +104,12 @@ HEAPgrow(MT_Lock *lock, Heap **hp, size_
*new = (Heap) {
.farmid = old->farmid,
.dirty = true,
- .remove = old->remove,
.parentid = old->parentid,
.wasempty = old->wasempty,
};
memcpy(new->filename, old->filename, sizeof(new->filename));
if (HEAPalloc(new, size, 1, 1) == GDK_SUCCEED) {
- ATOMIC_INIT(&new->refs, 1);
+ ATOMIC_INIT(&new->refs, 1 | (refs & HEAPREMOVE));
new->free = old->free;
new->cleanhash = old->cleanhash;
if (old->free > 0 &&
@@ -553,7 +553,6 @@ GDKupgradevarheap(BAT *b, var_t v, BUN c
*new = (Heap) {
.farmid = old->farmid,
.dirty = true,
- .remove = old->remove,
.parentid = old->parentid,
.wasempty = old->wasempty,
};
@@ -564,7 +563,7 @@ GDKupgradevarheap(BAT *b, var_t v, BUN c
}
/* HEAPalloc initialized .free, so we need to set it after */
new->free = old->free << (shift - b->tshift);
- ATOMIC_INIT(&new->refs, 1);
+ ATOMIC_INIT(&new->refs, 1 | (ATOMIC_GET(&old->refs) & HEAPREMOVE));
switch (width) {
case 1:
memcpy(new->base, old->base, n);
@@ -722,11 +721,13 @@ HEAPfree(Heap *h, bool rmheap)
void
HEAPdecref(Heap *h, bool remove)
{
- h->remove |= remove;
+ if (remove)
+ ATOMIC_OR(&h->refs, HEAPREMOVE);
+ ATOMIC_BASE_TYPE refs = ATOMIC_DEC(&h->refs);
//printf("dec ref(%d) %p %d\n", (int)h->refs, h, h->parentid);
- if (ATOMIC_DEC(&h->refs) == 0) {
+ if ((refs & HEAPREFS) == 0) {
ATOMIC_DESTROY(&h->refs);
- HEAPfree(h, h->remove);
+ HEAPfree(h, (bool) (refs & HEAPREMOVE));
GDKfree(h);
}
}
diff --git a/gdk/gdk_imprints.c b/gdk/gdk_imprints.c
--- a/gdk/gdk_imprints.c
+++ b/gdk/gdk_imprints.c
@@ -797,10 +797,12 @@ void
IMPSdecref(Imprints *imprints, bool remove)
{
TRC_DEBUG(ACCELERATOR, "Decrement ref count of %s\n",
imprints->imprints.filename);
- imprints->imprints.remove |= remove;
- if (ATOMIC_DEC(&imprints->imprints.refs) == 0) {
+ if (remove)
+ ATOMIC_OR(&imprints->imprints.refs, HEAPREMOVE);
+ ATOMIC_BASE_TYPE refs = ATOMIC_DEC(&imprints->imprints.refs);
+ if ((refs & HEAPREFS) == 0) {
ATOMIC_DESTROY(&imprints->imprints.refs);
- HEAPfree(&imprints->imprints, imprints->imprints.remove);
+ HEAPfree(&imprints->imprints, (bool) (refs & HEAPREMOVE));
GDKfree(imprints);
}
}
diff --git a/gdk/gdk_private.h b/gdk/gdk_private.h
--- a/gdk/gdk_private.h
+++ b/gdk/gdk_private.h
@@ -492,6 +492,9 @@ extern MT_Lock GDKtmLock;
#define GDKswapLock(x) GDKbatLock[(x)&BBP_BATMASK].swap
+#define HEAPREMOVE ((ATOMIC_BASE_TYPE) 1 << 63)
+#define HEAPREFS (((ATOMIC_BASE_TYPE) 1 << 63) - 1)
+
/* when the number of updates to a BAT is less than 1 in this number, we
* keep the unique_est property */
#define GDK_UNIQUE_ESTIMATE_KEEP_FRACTION 1000
diff --git a/gdk/gdk_strimps.c b/gdk/gdk_strimps.c
--- a/gdk/gdk_strimps.c
+++ b/gdk/gdk_strimps.c
@@ -783,12 +783,14 @@ STRMPcreate(BAT *b, BAT *s)
void
STRMPdecref(Strimps *strimps, bool remove)
{
+ if (remove)
+ ATOMIC_OR(&strimps->strimps.refs, HEAPREMOVE);
+ ATOMIC_BASE_TYPE refs = ATOMIC_DEC(&strimps->strimps.refs);
TRC_DEBUG(ACCELERATOR, "Decrement ref count of %s to " BUNFMT "\n",
- strimps->strimps.filename, (BUN)
(ATOMIC_GET(&strimps->strimps.refs) - 1));
- strimps->strimps.remove |= remove;
- if (ATOMIC_DEC(&strimps->strimps.refs) == 0) {
+ strimps->strimps.filename, (BUN) (refs & HEAPREFS));
+ if ((refs & HEAPREFS) == 0) {
ATOMIC_DESTROY(&strimps->strimps.refs);
- HEAPfree(&strimps->strimps, strimps->strimps.remove);
+ HEAPfree(&strimps->strimps, (bool) (refs & HEAPREMOVE));
GDKfree(strimps);
}
@@ -797,10 +799,9 @@ STRMPdecref(Strimps *strimps, bool remov
void
STRMPincref(Strimps *strimps)
{
+ ATOMIC_BASE_TYPE refs = ATOMIC_INC(&strimps->strimps.refs);
TRC_DEBUG(ACCELERATOR, "Increment ref count of %s to " BUNFMT "\n",
- strimps->strimps.filename, (BUN)
(ATOMIC_GET(&strimps->strimps.refs) + 1));
- (void)ATOMIC_INC(&strimps->strimps.refs);
-
+ strimps->strimps.filename, (BUN) (refs & HEAPREFS));
}
void
_______________________________________________
checkin-list mailing list -- [email protected]
To unsubscribe send an email to [email protected]
