Changeset: 21e4ab0b2411 for MonetDB URL: https://dev.monetdb.org/hg/MonetDB/rev/21e4ab0b2411 Modified Files: gdk/ChangeLog.Sep2022 gdk/gdk_bat.c gdk/gdk_batop.c sql/ChangeLog.Sep2022 Branch: Sep2022 Log Message:
Merge with Jan2022 branch. diffs (205 lines): diff --git a/gdk/ChangeLog.Sep2022 b/gdk/ChangeLog.Sep2022 --- a/gdk/ChangeLog.Sep2022 +++ b/gdk/ChangeLog.Sep2022 @@ -1,6 +1,12 @@ # ChangeLog file for GDK # This file is updated with Maddlog +* Wed Dec 14 2022 Sjoerd Mullender <[email protected]> +- Fixed a race condition where a hash could have been created on a + bat using the old bat count while in another thread the bat count + got updated. This would make the hash be based on too small a size, + causing failures later on. + * Thu Dec 8 2022 Sjoerd Mullender <[email protected]> - When extending a bat failed, the capacity had been updated already and was therefore too large. This could then later cause a crash. This has diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c --- a/gdk/gdk_bat.c +++ b/gdk/gdk_bat.c @@ -1170,6 +1170,7 @@ BUNappendmulti(BAT *b, const void *value } BATiter bi = bat_iterator_nolock(b); MT_lock_unset(&b->theaplock); + MT_rwlock_wrlock(&b->thashlock); if (values && b->ttype) { int (*atomcmp) (const void *, const void *) = ATOMcompare(b->ttype); const void *atomnil = ATOMnilptr(b->ttype); @@ -1184,6 +1185,7 @@ BUNappendmulti(BAT *b, const void *value t = ((void **) values)[i]; gdk_return rc = tfastins_nocheckVAR(b, p, t); if (rc != GDK_SUCCEED) { + MT_rwlock_wrunlock(&b->thashlock); return rc; } if (vbase != b->tvheap->base) { @@ -1224,7 +1226,6 @@ BUNappendmulti(BAT *b, const void *value } p++; } - MT_rwlock_wrlock(&b->thashlock); if (b->thash) { p -= count; for (BUN i = 0; i < count; i++) { @@ -1234,7 +1235,6 @@ BUNappendmulti(BAT *b, const void *value } nunique = b->thash ? b->thash->nunique : 0; } - MT_rwlock_wrunlock(&b->thashlock); } else if (ATOMstorage(b->ttype) == TYPE_msk) { bi.minpos = bi.maxpos = BUN_NONE; minvalp = maxvalp = NULL; @@ -1248,7 +1248,6 @@ BUNappendmulti(BAT *b, const void *value minvalp = BUNtloc(bi, bi.minpos); if (bi.maxpos != BUN_NONE) maxvalp = BUNtloc(bi, bi.maxpos); - MT_rwlock_wrlock(&b->thashlock); for (BUN i = 0; i < count; i++) { t = (void *) ((char *) values + (i << b->tshift)); gdk_return rc = tfastins_nocheckFIX(b, p, t); @@ -1279,14 +1278,12 @@ BUNappendmulti(BAT *b, const void *value p++; } nunique = b->thash ? b->thash->nunique : 0; - MT_rwlock_wrunlock(&b->thashlock); } MT_lock_set(&b->theaplock); b->tminpos = bi.minpos; b->tmaxpos = bi.maxpos; MT_lock_unset(&b->theaplock); } else { - MT_rwlock_wrlock(&b->thashlock); for (BUN i = 0; i < count; i++) { gdk_return rc = tfastins_nocheck(b, p, t); if (rc != GDK_SUCCEED) { @@ -1299,13 +1296,13 @@ BUNappendmulti(BAT *b, const void *value p++; } nunique = b->thash ? b->thash->nunique : 0; - MT_rwlock_wrunlock(&b->thashlock); } MT_lock_set(&b->theaplock); BATsetcount(b, p); if (nunique != 0) b->tunique_est = (double) nunique; MT_lock_unset(&b->theaplock); + MT_rwlock_wrunlock(&b->thashlock); IMPSdestroy(b); /* no support for inserts in imprints yet */ OIDXdestroy(b); @@ -1581,14 +1578,11 @@ BUNinplacemulti(BAT *b, const oid *posit } if (b->twidth < SIZEOF_VAR_T && (b->twidth <= 2 ? _d - GDK_VAROFFSET : _d) >= ((size_t) 1 << (8 << b->tshift))) { - /* doesn't fit in current heap, upgrade - * it, can't keep hashlock while doing - * so */ - MT_rwlock_wrunlock(&b->thashlock); + /* doesn't fit in current heap, upgrade it */ if (GDKupgradevarheap(b, _d, 0, bi.count) != GDK_SUCCEED) { + MT_rwlock_wrunlock(&b->thashlock); return GDK_FAIL; } - MT_rwlock_wrlock(&b->thashlock); } /* reinitialize iterator after possible heap upgrade */ { diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c --- a/gdk/gdk_batop.c +++ b/gdk/gdk_batop.c @@ -307,12 +307,12 @@ insert_string_bat(BAT *b, BATiter *ni, s } } TIMEOUT_CHECK(timeoffset, TIMEOUT_HANDLER(GDK_FAIL)); + MT_rwlock_wrlock(&b->thashlock); MT_lock_set(&b->theaplock); BATsetcount(b, oldcnt + ci->ncand); assert(b->batCapacity >= b->batCount); MT_lock_unset(&b->theaplock); /* maintain hash */ - MT_rwlock_wrlock(&b->thashlock); for (r = oldcnt, cnt = BATcount(b); b->thash && r < cnt; r++) { HASHappend_locked(b, r, b->tvheap->base + VarHeapVal(Tloc(b, 0), r, b->twidth)); } @@ -386,11 +386,11 @@ append_varsized_bat(BAT *b, BATiter *ni, *dst++ = src[canditer_next(ci) - hseq]; } } + MT_rwlock_wrlock(&b->thashlock); MT_lock_set(&b->theaplock); BATsetcount(b, BATcount(b) + ci->ncand); MT_lock_unset(&b->theaplock); /* maintain hash table */ - MT_rwlock_wrlock(&b->thashlock); for (BUN i = BATcount(b) - ci->ncand; b->thash && i < BATcount(b); i++) { @@ -482,12 +482,12 @@ append_varsized_bat(BAT *b, BATiter *ni, } } BUN nunique = b->thash ? b->thash->nunique : 0; - MT_rwlock_wrunlock(&b->thashlock); MT_lock_set(&b->theaplock); BATsetcount(b, r); if (nunique != 0) b->tunique_est = (double) nunique; MT_lock_unset(&b->theaplock); + MT_rwlock_wrunlock(&b->thashlock); return GDK_SUCCEED; } @@ -909,12 +909,12 @@ BATappend2(BAT *b, BAT *n, BAT *s, bool } BUN nunique; nunique = b->thash ? b->thash->nunique : 0; - MT_rwlock_wrunlock(&b->thashlock); MT_lock_set(&b->theaplock); BATsetcount(b, b->batCount + ci.ncand); if (nunique != 0) b->tunique_est = (double) nunique; MT_lock_unset(&b->theaplock); + MT_rwlock_wrunlock(&b->thashlock); } doreturn: @@ -1324,16 +1324,10 @@ BATappend_or_update(BAT *b, BAT *p, cons } if (b->twidth < SIZEOF_VAR_T && (b->twidth <= 2 ? d - GDK_VAROFFSET : d) >= ((size_t) 1 << (8 << b->tshift))) { - /* doesn't fit in current heap, upgrade - * it, can't keep hashlock while doing - * so */ - MT_rwlock_wrunlock(&b->thashlock); - locked = false; + /* doesn't fit in current heap, upgrade it */ if (GDKupgradevarheap(b, d, 0, MAX(updid, b->batCount)) != GDK_SUCCEED) { goto bailout; } - MT_rwlock_wrlock(&b->thashlock); - locked = true; } /* in case ATOMreplaceVAR and/or * GDKupgradevarheap replaces a heap, we need to @@ -1377,6 +1371,7 @@ BATappend_or_update(BAT *b, BAT *p, cons b->tvheap->dirty = true; MT_lock_unset(&b->theaplock); } else if (ATOMstorage(b->ttype) == TYPE_msk) { + assert(b->thash == NULL); HASHdestroy(b); /* hash doesn't make sense for msk */ for (BUN i = 0; i < ni.count; i++) { oid updid; diff --git a/sql/ChangeLog.Sep2022 b/sql/ChangeLog.Sep2022 --- a/sql/ChangeLog.Sep2022 +++ b/sql/ChangeLog.Sep2022 @@ -1,6 +1,11 @@ # ChangeLog file for sql # This file is updated with Maddlog +* Wed Dec 14 2022 Sjoerd Mullender <[email protected]> +- Fixed cleanup after a failed allocation where the data being cleaned + up was unitialized but still used as pointers to memory that also had + to be freed. + * Wed Dec 7 2022 Sjoerd Mullender <[email protected]> - Fixed a double cleanup after a failed allocation in COPY INTO. The double cleanup could cause a crash due to a race condition it enabled. _______________________________________________ checkin-list mailing list -- [email protected] To unsubscribe send an email to [email protected]
