Changeset: d9d408abb9c4 for MonetDB URL: https://dev.monetdb.org/hg/MonetDB/rev/d9d408abb9c4 Modified Files: gdk/gdk.h gdk/gdk_bat.c gdk/gdk_batop.c gdk/gdk_bbp.c gdk/gdk_calc.c gdk/gdk_hash.c gdk/gdk_heap.c gdk/gdk_imprints.c gdk/gdk_logger.c gdk/gdk_orderidx.c gdk/gdk_private.h gdk/gdk_project.c gdk/gdk_sample.c gdk/gdk_select.c gdk/gdk_storage.c gdk/gdk_strimps.c monetdb5/mal/mal_debugger.c monetdb5/modules/kernel/bat5.c monetdb5/modules/mal/remote.c Branch: default Log Message:
Fixing data races. diffs (truncated from 1430 to 300 lines): diff --git a/gdk/gdk.h b/gdk/gdk.h --- a/gdk/gdk.h +++ b/gdk/gdk.h @@ -792,17 +792,16 @@ typedef struct BAT { oid hseqbase; /* head seq base */ MT_Id creator_tid; /* which thread created it */ bat batCacheid; /* index into BBP */ + role_t batRole; /* role of the bat */ /* dynamic bat properties */ - restrict_t batRestricted; /* access privileges */ - bool batTransient; /* should the BAT persist on disk? */ + restrict_t batRestricted:2; /* access privileges */ bool + batTransient:1, /* should the BAT persist on disk? */ batCopiedtodisk:1, /* once written */ batDirtyflushed:1, /* was dirty before commit started? */ batDirtydesc:1; /* bat descriptor dirty marker */ - uint16_t /* adjacent bit fields are packed together (if they fit) */ - selcnt:10; /* how often used in equi select without hash */ - role_t batRole; /* role of the bat */ + uint16_t selcnt; /* how often used in equi select without hash */ uint16_t unused; /* value=0 for now (sneakily used by mat.c) */ int batSharecnt; /* incoming view count */ @@ -979,13 +978,13 @@ typedef struct BATiter { copiedtodisk:1, transient:1; restrict_t restricted:2; +#ifndef NDEBUG + bool locked:1; +#endif union { oid tvid; bool tmsk; }; -#ifndef NDEBUG - bool locked; -#endif } BATiter; static inline BATiter diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c --- a/gdk/gdk_bat.c +++ b/gdk/gdk_bat.c @@ -804,7 +804,7 @@ COLcopy(BAT *b, int tt, bool writable, r * atom-types */ if (!writable && role == TRANSIENT && - b->batRestricted == BAT_READ && + bi.restricted == BAT_READ && ATOMstorage(b->ttype) != TYPE_msk && /* no view on TYPE_msk */ (!VIEWtparent(b) || BBP_cache(VIEWtparent(b))->batRestricted == BAT_READ)) { @@ -1064,11 +1064,10 @@ BUNappendmulti(BAT *b, const void *value return rc; } - if (count > BATcount(b) / gdk_unique_estimate_keep_fraction) { - MT_lock_set(&b->theaplock); + MT_lock_set(&b->theaplock); + if (count > BATcount(b) / gdk_unique_estimate_keep_fraction) b->tunique_est = 0; - MT_lock_unset(&b->theaplock); - } + const void *t = b->ttype == TYPE_msk ? &(msk){false} : ATOMnilptr(b->ttype); if (b->ttype == TYPE_oid) { /* spend extra effort on oid (possible candidate list) */ @@ -1164,6 +1163,7 @@ BUNappendmulti(BAT *b, const void *value b->tnonil = false; b->tsorted = b->trevsorted = b->tkey = false; } + MT_lock_unset(&b->theaplock); MT_rwlock_wrlock(&b->thashlock); if (values && b->ttype) { int (*atomcmp) (const void *, const void *) = ATOMcompare(b->ttype); @@ -1324,7 +1324,7 @@ BUNdelete(BAT *b, oid o) MT_lock_unset(&b->theaplock); if (ATOMunfix(b->ttype, val) != GDK_SUCCEED) return GDK_FAIL; - HASHdelete(b, p, val); + HASHdelete(&bi, p, val); ATOMdel(b->ttype, b->tvheap, (var_t *) BUNtloc(bi, p)); if (p != BATcount(b) - 1 && (b->ttype != TYPE_void || BATtdense(b))) { @@ -1341,9 +1341,9 @@ BUNdelete(BAT *b, oid o) mskClr(b, BATcount(b) - 1); } else { val = Tloc(b, BATcount(b) - 1); - HASHdelete(b, BATcount(b) - 1, val); + HASHdelete(&bi, BATcount(b) - 1, val); memcpy(Tloc(b, p), val, b->twidth); - HASHinsert(b, p, val); + HASHinsert(&bi, p, val); MT_lock_set(&b->theaplock); if (b->tminpos == BATcount(b) - 1) b->tminpos = p; @@ -1493,7 +1493,7 @@ BUNinplacemulti(BAT *b, const oid *posit } } } - HASHdelete_locked(b, p, val); /* first delete old value from hash */ + HASHdelete_locked(&bi, p, val); /* first delete old value from hash */ } else { /* out of range old value, so the properties and * hash cannot be trusted */ @@ -1612,7 +1612,7 @@ BUNinplacemulti(BAT *b, const oid *posit } } - HASHinsert_locked(b, p, t); /* insert new value into hash */ + HASHinsert_locked(&bi, p, t); /* insert new value into hash */ tt = b->ttype; prv = p > 0 ? p - 1 : BUN_NONE; @@ -2344,7 +2344,10 @@ restrict_t BATgetaccess(BAT *b) { BATcheck(b, BAT_WRITE); - return b->batRestricted; + MT_lock_set(&b->theaplock); + restrict_t restricted = b->batRestricted; + MT_lock_unset(&b->theaplock); + return restricted; } /* @@ -2369,19 +2372,6 @@ BATgetaccess(BAT *b) * the heap modes until the commit point is reached. So we do not need * to do anything with heap modes yet at this point. */ -#define check_type(tp) \ - do { \ - if (ATOMisdescendant((tp), TYPE_ptr) || \ - BATatoms[tp].atomUnfix || \ - BATatoms[tp].atomFix) { \ - GDKerror("%s type implies that %s[%s] " \ - "cannot be made persistent.\n", \ - ATOMname(tp), BATgetId(b), \ - ATOMname(b->ttype)); \ - return GDK_FAIL; \ - } \ - } while (0) - gdk_return BATmode(BAT *b, bool transient) { @@ -2398,17 +2388,28 @@ BATmode(BAT *b, bool transient) return GDK_FAIL; } - if (transient != b->batTransient) { + BATiter bi = bat_iterator(b); + + if (transient != bi.transient) { bat bid = b->batCacheid; if (!transient) { - check_type(b->ttype); + if (ATOMisdescendant(b->ttype, TYPE_ptr) || + BATatoms[b->ttype].atomUnfix || + BATatoms[b->ttype].atomFix) { + GDKerror("%s type implies that %s[%s] " + "cannot be made persistent.\n", + ATOMname(b->ttype), BATgetId(b), + ATOMname(b->ttype)); + bat_iterator_end(&bi); + return GDK_FAIL; + } } /* persistent BATs get a logical reference */ if (!transient) { BBPretain(bid); - } else if (!b->batTransient) { + } else if (!bi.transient) { BBPrelease(bid); } MT_lock_set(&GDKswapLock(bid)); @@ -2418,7 +2419,7 @@ BATmode(BAT *b, bool transient) BBP_status_off(bid, BBPDELETED); } else BBP_status_on(bid, BBPNEW); - } else if (!b->batTransient) { + } else if (!bi.transient) { if (!(BBP_status(bid) & BBPNEW)) BBP_status_on(bid, BBPDELETED); BBP_status_off(bid, BBPPERSISTENT); @@ -2439,6 +2440,7 @@ BATmode(BAT *b, bool transient) MT_lock_unset(&b->theaplock); MT_lock_unset(&GDKswapLock(bid)); } + bat_iterator_end(&bi); return GDK_SUCCEED; } diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c --- a/gdk/gdk_batop.c +++ b/gdk/gdk_batop.c @@ -344,7 +344,7 @@ append_varsized_bat(BAT *b, BATiter *ni, if (mayshare && BATcount(b) == 0 && b->batRole == TRANSIENT && - ni->b->batRestricted == BAT_READ && + ni->restricted == BAT_READ && b->tvheap != ni->vh) { /* if b is still empty, in the transient farm, and n * is read-only, we replace b's vheap with a reference @@ -898,7 +898,9 @@ BATdel(BAT *b, BAT *d) { gdk_return (*unfix) (const void *) = BATatoms[b->ttype].atomUnfix; void (*atmdel) (Heap *, var_t *) = BATatoms[b->ttype].atomDel; + MT_lock_set(&b->theaplock); BATiter bi = bat_iterator_nolock(b); + MT_lock_unset(&b->theaplock); assert(ATOMtype(d->ttype) == TYPE_oid); assert(d->tsorted); @@ -960,7 +962,6 @@ BATdel(BAT *b, BAT *d) // o += b->hseqbase; // if this were to be used again } b->batCount -= c; - MT_lock_unset(&b->theaplock); } else { BATiter di = bat_iterator(d); const oid *o = (const oid *) di.base; @@ -1031,7 +1032,6 @@ BATdel(BAT *b, BAT *d) MT_lock_set(&b->theaplock); b->theap->dirty = true; b->batCount -= nd; - MT_lock_unset(&b->theaplock); } if (b->batCount <= 1) { /* some trivial properties */ @@ -1043,7 +1043,6 @@ BATdel(BAT *b, BAT *d) } } /* not sure about these anymore */ - MT_lock_set(&b->theaplock); b->tnosorted = b->tnorevsorted = 0; b->tnokey[0] = b->tnokey[1] = 0; b->tminpos = BUN_NONE; @@ -1101,18 +1100,19 @@ BATappend_or_update(BAT *b, BAT *p, cons if (BATcount(n) == 0) { return GDK_SUCCEED; } - if (!force && (b->batRestricted != BAT_WRITE || b->batSharecnt > 0)) { - GDKerror("access denied to %s, aborting.\n", BATgetId(b)); - return GDK_FAIL; - } - BATiter bi = bat_iterator_nolock(b); BATiter ni = bat_iterator(n); OIDXdestroy(b); IMPSdestroy(b); STRMPdestroy(b); MT_lock_set(&b->theaplock); + if (!force && (b->batRestricted != BAT_WRITE || b->batSharecnt > 0)) { + MT_lock_unset(&b->theaplock); + GDKerror("access denied to %s, aborting.\n", BATgetId(b)); + return GDK_FAIL; + } + BATiter bi = bat_iterator_nolock(b); if (ni.count > BATcount(b) / gdk_unique_estimate_keep_fraction) { b->tunique_est = 0; } @@ -1237,7 +1237,7 @@ BATappend_or_update(BAT *b, BAT *p, cons MT_rwlock_wrlock(&b->thashlock); locked = true; } - HASHdelete_locked(b, updid, old); + HASHdelete_locked(&bi, updid, old); var_t d; switch (b->twidth) { @@ -1297,7 +1297,7 @@ BATappend_or_update(BAT *b, BAT *p, cons default: MT_UNREACHABLE(); } - HASHinsert_locked(b, updid, new); + HASHinsert_locked(&bi, updid, new); } if (locked) { @@ -1387,7 +1387,7 @@ BATappend_or_update(BAT *b, BAT *p, cons MT_rwlock_wrlock(&b->thashlock); locked = true; for (BUN i = pos, j = pos + ni.count; i < j && b->thash; i++) - HASHdelete_locked(b, i, Tloc(b, i)); + HASHdelete_locked(&bi, i, Tloc(b, i)); if (ni.type == TYPE_void) { assert(b->ttype == TYPE_oid); oid *o = Tloc(b, pos); @@ -1454,7 +1454,7 @@ BATappend_or_update(BAT *b, BAT *p, cons * been destroyed above */ if (b->thash != NULL) { for (BUN i = pos, j = pos + ni.count; i < j; i++) - HASHinsert_locked(b, i, Tloc(b, i)); _______________________________________________ checkin-list mailing list -- checkin-list@monetdb.org To unsubscribe send an email to checkin-list-le...@monetdb.org