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

Reply via email to