Changeset: 529d9dd14709 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/529d9dd14709
Modified Files:
clients/Tests/exports.stable.out
gdk/gdk.h
gdk/gdk_align.c
gdk/gdk_bat.c
gdk/gdk_bbp.c
gdk/gdk_bbp.h
gdk/gdk_hash.c
gdk/gdk_private.h
sql/storage/bat/bat_storage.c
Branch: default
Log Message:
Fixed a bunch of data races.
diffs (truncated from 357 to 300 lines):
diff --git a/clients/Tests/exports.stable.out b/clients/Tests/exports.stable.out
--- a/clients/Tests/exports.stable.out
+++ b/clients/Tests/exports.stable.out
@@ -115,6 +115,7 @@ BAT *BATconvert(BAT *b, BAT *s, int tp,
BUN BATcount_no_nil(BAT *b, BAT *s);
gdk_return BATdel(BAT *b, BAT *d) __attribute__((__warn_unused_result__));
BAT *BATdense(oid hseq, oid tseq, BUN cnt)
__attribute__((__warn_unused_result__));
+BAT *BATdescriptor(bat i);
BAT *BATdiff(BAT *l, BAT *r, BAT *sl, BAT *sr, bool nil_matches, bool not_in,
BUN estimate);
BAT *BATdiffcand(BAT *a, BAT *b);
BAT *BATdiffcand(BAT *a, BAT *b);
@@ -208,7 +209,6 @@ gdk_return BATupdatepos(BAT *b, const oi
BBPrec *BBP[N_BBPINIT];
gdk_return BBPaddfarm(const char *dirname, uint32_t rolemask, bool logerror);
void BBPcold(bat i);
-BAT *BBPdescriptor(bat b);
int BBPfix(bat b);
bat BBPindex(const char *nme);
void BBPkeepref(BAT *b) __attribute__((__nonnull__(1)));
diff --git a/gdk/gdk.h b/gdk/gdk.h
--- a/gdk/gdk.h
+++ b/gdk/gdk.h
@@ -1895,20 +1895,7 @@ BBPcheck(bat x)
return 0;
}
-static inline BAT *
-BATdescriptor(bat i)
-{
- BAT *b = NULL;
-
- if (BBPcheck(i)) {
- if (BBPfix(i) <= 0)
- return NULL;
- b = BBP_cache(i);
- if (b == NULL)
- b = BBPdescriptor(i);
- }
- return b;
-}
+gdk_export BAT *BATdescriptor(bat i);
static inline void *
Tpos(BATiter *bi, BUN p)
diff --git a/gdk/gdk_align.c b/gdk/gdk_align.c
--- a/gdk/gdk_align.c
+++ b/gdk/gdk_align.c
@@ -96,15 +96,35 @@ VIEWcreate(oid seq, BAT *b)
return NULL;
assert(bn->theap == NULL);
- /* the T column descriptor is fully copied. We need copies
- * because in case of a mark, we are going to override a
- * column with a void. Take care to zero the accelerator data,
- * though. */
MT_lock_set(&b->theaplock);
bn->batInserted = b->batInserted;
bn->batCount = b->batCount;
bn->batCapacity = b->batCapacity;
- bn->T = b->T;
+ bn->batRestricted = BAT_READ;
+
+ /* the T column descriptor is fully copied except for the
+ * accelerator data. We need copies because in case of a mark,
+ * we are going to override a column with a void. */
+ bn->tkey = b->tkey;
+ bn->tvarsized = b->tvarsized;
+ bn->tseqbase = b->tseqbase;
+ bn->tsorted = b->tsorted;
+ bn->trevsorted = b->trevsorted;
+ bn->twidth = b->twidth;
+ bn->tshift = b->tshift;
+ bn->tnonil = b->tnonil;
+ bn->tnil = b->tnil;
+ bn->tnokey[0] = b->tnokey[0];
+ bn->tnokey[1] = b->tnokey[1];
+ bn->tnosorted = b->tnosorted;
+ bn->tnorevsorted = b->tnorevsorted;
+ bn->tminpos = b->tminpos;
+ bn->tmaxpos = b->tmaxpos;
+ bn->tunique_est = b->tunique_est;
+ bn->theap = b->theap;
+ bn->tbaseoff = b->tbaseoff;
+ bn->tvheap = b->tvheap;
+
tp = VIEWtparent(b);
if (tp == 0 && b->ttype != TYPE_void)
tp = b->batCacheid;
@@ -121,18 +141,6 @@ VIEWcreate(oid seq, BAT *b)
BBPshare(bn->tvheap->parentid);
}
- bn->tprops = NULL;
-
- /* correct values after copy of column info */
- BATinit_idents(bn);
- bn->batRestricted = BAT_READ;
- bn->thash = NULL;
- /* imprints are shared, but the check is dynamic */
- bn->timprints = NULL;
- /* Order OID index */
- bn->torderidx = NULL;
- /* Only the parent should have a pointer to the strimp */
- bn->tstrimps = NULL;
if (BBPcacheit(bn, true) != GDK_SUCCEED) { /* enter in BBP */
if (tp) {
BBPunshare(tp);
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -1067,7 +1067,6 @@ BUNappendmulti(BAT *b, const void *value
b->tunique_est = 0;
MT_lock_unset(&b->theaplock);
}
- b->theap->dirty = true;
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) */
diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c
--- a/gdk/gdk_bbp.c
+++ b/gdk/gdk_bbp.c
@@ -122,8 +122,10 @@ static gdk_return BBPbackup(BAT *b, bool
static gdk_return BBPdir_init(void);
static void BBPcallbacks(void);
-static lng BBPlogno; /* two lngs of extra info in BBP.dir */
-static lng BBPtransid;
+/* two lngs of extra info in BBP.dir */
+/* these two need to be atomic because of their use in AUTHcommit() */
+static ATOMIC_TYPE BBPlogno = ATOMIC_VAR_INIT(0);
+static ATOMIC_TYPE BBPtransid = ATOMIC_VAR_INIT(0);
#define BBPtmpcheck(s) (strncmp(s, "tmp_", 4) == 0)
@@ -162,13 +164,13 @@ getBBPsize(void)
lng
getBBPlogno(void)
{
- return BBPlogno;
+ return (lng) ATOMIC_GET(&BBPlogno);
}
lng
getBBPtransid(void)
{
- return BBPtransid;
+ return (lng) ATOMIC_GET(&BBPtransid);
}
@@ -995,10 +997,13 @@ BBPheader(FILE *fp, int *lineno, bat *bb
TRC_CRITICAL(GDK, "short BBP");
return 0;
}
- if (sscanf(buf, "BBPinfo=" LLSCN " " LLSCN, &BBPlogno,
&BBPtransid) != 2) {
+ lng logno, transid;
+ if (sscanf(buf, "BBPinfo=" LLSCN " " LLSCN, &logno, &transid)
!= 2) {
TRC_CRITICAL(GDK, "no info value found\n");
return 0;
}
+ ATOMIC_SET(&BBPlogno, logno);
+ ATOMIC_SET(&BBPtransid, transid);
}
return bbpversion;
}
@@ -2769,48 +2774,16 @@ BBPcold(bat i)
}
/* This function can fail if the input parameter (i) is incorrect
- * (unlikely), or, if the bat is a view, this is a physical (not
- * logical) incref (i.e. called through BBPfix(), and it is the first
- * reference (refs was 0 and should become 1). It can fail in this
- * case if the parent bat cannot be loaded.
- * This means the return value of BBPfix should be checked in these
- * circumstances, but not necessarily in others. */
+ * (unlikely). */
static inline int
incref(bat i, bool logical, bool lock)
{
int refs;
- bat tp = i, tvp = i;
BAT *b, *pb = NULL, *pvb = NULL;
if (!BBPcheck(i))
return 0;
- /* Before we get the lock and before we do all sorts of
- * things, make sure we can load the parent bats if there are
- * any. If we can't load them, we can still easily fail. If
- * this is indeed a view, but not the first physical
- * reference, getting the parent BAT descriptor is
- * superfluous, but not too expensive, so we do it anyway. */
- if (!logical && (b = BBP_desc(i)) != NULL) {
- MT_lock_set(&b->theaplock);
- tp = b->theap ? b->theap->parentid : i;
- tvp = b->tvheap ? b->tvheap->parentid : i;
- MT_lock_unset(&b->theaplock);
- if (tp != i) {
- pb = BATdescriptor(tp);
- if (pb == NULL)
- return 0;
- }
- if (tvp != i) {
- pvb = BATdescriptor(tvp);
- if (pvb == NULL) {
- if (pb)
- BBPunfix(pb->batCacheid);
- return 0;
- }
- }
- }
-
if (lock) {
for (;;) {
MT_lock_set(&GDKswapLock(i));
@@ -2834,11 +2807,9 @@ incref(bat i, bool logical, bool lock)
assert(BBP_refs(i) + BBP_lrefs(i) ||
BBP_status(i) & (BBPDELETED | BBPSWAPPED));
if (logical) {
- /* parent BATs are not relevant for logical refs */
refs = ++BBP_lrefs(i);
BBP_pid(i) = 0;
} else {
- assert(tp >= 0);
refs = ++BBP_refs(i);
BBP_status_on(i, BBPHOT);
}
@@ -2856,12 +2827,52 @@ incref(bat i, bool logical, bool lock)
return refs;
}
+BAT *
+BATdescriptor(bat i)
+{
+ BAT *b = NULL;
+
+ if (BBPcheck(i)) {
+ b = BBP_desc(i);
+ if (b->theap->parentid != b->batCacheid &&
+ BATdescriptor(b->theap->parentid) == NULL)
+ return NULL;
+ if (b->tvheap &&
+ b->tvheap->parentid != b->batCacheid &&
+ BATdescriptor(b->tvheap->parentid) == NULL) {
+ if (b->theap->parentid != b->batCacheid)
+ BBPunfix(b->theap->parentid);
+ return NULL;
+ }
+ for (;;) {
+ MT_lock_set(&GDKswapLock(i));
+ if (!(BBP_status(i) & (BBPUNSTABLE|BBPLOADING)))
+ break;
+ /* the BATs is "unstable", try again */
+ MT_lock_unset(&GDKswapLock(i));
+ BBPspin(i, __func__, BBPUNSTABLE|BBPLOADING);
+ }
+ if (incref(i, false, false) <= 0)
+ return NULL;
+ b = BBP_cache(i);
+ if (b == NULL)
+ b = getBBPdescriptor(i, false);
+ MT_lock_unset(&GDKswapLock(i));
+ }
+ return b;
+}
+
/* see comment for incref */
int
BBPfix(bat i)
{
bool lock = locked_by == 0 || locked_by != MT_getpid();
+ BAT *b = BBP_desc(i);
+ if (b->theap->parentid != b->batCacheid)
+ (void) BBPfix(b->theap->parentid);
+ if (b->tvheap && b->tvheap->parentid != b->batCacheid)
+ (void) BBPfix(b->tvheap->parentid);
return incref(i, false, lock);
}
@@ -2886,6 +2897,7 @@ BBPshare(bat parent)
assert(BBP_refs(parent) > 0);
if (lock)
MT_lock_unset(&GDKswapLock(parent));
+ assert(!isVIEW(BBP_desc(parent)));
(void) incref(parent, false, lock);
}
@@ -3941,8 +3953,8 @@ BBPsync(int cnt, bat *restrict subcommit
/* AFTERMATH */
if (ret == GDK_SUCCEED) {
- BBPlogno = logno; /* the new value */
- BBPtransid = transid;
+ ATOMIC_SET(&BBPlogno, logno); /* the new value */
+ ATOMIC_SET(&BBPtransid, transid);
backup_files = subcommit ? (backup_files - backup_subdir) : 0;
backup_dir = backup_subdir = 0;
if (GDKremovedir(0, DELDIR) != GDK_SUCCEED)
diff --git a/gdk/gdk_bbp.h b/gdk/gdk_bbp.h
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org
