Changeset: 48266ba700bb for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/48266ba700bb
Modified Files:
gdk/gdk.h
gdk/gdk_atoms.c
gdk/gdk_bbp.c
gdk/gdk_private.h
gdk/gdk_utils.c
monetdb5/mal/mal.c
monetdb5/mal/mal_prelude.c
monetdb5/mal/mal_profiler.c
monetdb5/modules/mal/tablet.c
sql/storage/store.c
tools/mserver/mserver5.c
Branch: default
Log Message:
Fixed a bunch of data races and a lock order inversion.
Starting with a new database and immediately stopping now causes no data
races.
diffs (truncated from 372 to 300 lines):
diff --git a/gdk/gdk.h b/gdk/gdk.h
--- a/gdk/gdk.h
+++ b/gdk/gdk.h
@@ -567,10 +567,10 @@ typedef struct {
ATOMIC_TYPE refs; /* reference count for this heap */
bte farmid; /* id of farm where heap is located */
- bool cleanhash:1, /* string heaps must clean hash */
- dirty:1, /* specific heap dirty marker */
- remove:1, /* remove storage file when freeing */
- wasempty:1; /* heap was empty when last saved/created */
+ bool cleanhash; /* string heaps must clean hash */
+ bool dirty; /* specific heap dirty marker */
+ bool remove; /* remove storage file when freeing */
+ bool wasempty; /* heap was empty when last saved/created */
storage_t storage; /* storage mode (mmap/malloc). */
storage_t newstorage; /* new desired storage mode at re-allocation. */
bat parentid; /* cache id of VIEW parent bat */
@@ -762,6 +762,18 @@ typedef struct {
/* if the version number is updated, also fix snapshot_bats() in bat_logger.c
*/
#define GDKLIBRARY 061046U /* first after Jan2022 */
+/* The batRestricted field indicates whether a BAT is readonly.
+ * we have modes: BAT_WRITE = all permitted
+ * BAT_APPEND = append-only
+ * BAT_READ = read-only
+ * VIEW bats are always mapped read-only.
+ */
+typedef enum {
+ BAT_WRITE, /* all kinds of access allowed */
+ BAT_READ, /* only read-access allowed */
+ BAT_APPEND, /* only reads and appends allowed */
+} restrict_t;
+
typedef struct BAT {
/* static bat properties */
oid hseqbase; /* head seq base */
@@ -769,13 +781,12 @@ typedef struct BAT {
bat batCacheid; /* index into BBP */
/* dynamic bat properties */
+ restrict_t batRestricted; /* access privileges */
+ bool batTransient; /* should the BAT persist on disk? */
bool
batCopiedtodisk:1, /* once written */
batDirtyflushed:1, /* was dirty before commit started? */
- batDirtydesc:1, /* bat descriptor dirty marker */
- batTransient:1; /* should the BAT persist on disk? */
- uint8_t /* adjacent bit fields are packed together (if they fit) */
- batRestricted:2; /* access privileges */
+ 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 */
@@ -1232,18 +1243,6 @@ gdk_export gdk_return BATroles(BAT *b, c
gdk_export void BAThseqbase(BAT *b, oid o);
gdk_export void BATtseqbase(BAT *b, oid o);
-/* The batRestricted field indicates whether a BAT is readonly.
- * we have modes: BAT_WRITE = all permitted
- * BAT_APPEND = append-only
- * BAT_READ = read-only
- * VIEW bats are always mapped read-only.
- */
-typedef enum {
- BAT_WRITE, /* all kinds of access allowed */
- BAT_READ, /* only read-access allowed */
- BAT_APPEND, /* only reads and appends allowed */
-} restrict_t;
-
gdk_export BAT *BATsetaccess(BAT *b, restrict_t mode)
__attribute__((__warn_unused_result__));
gdk_export restrict_t BATgetaccess(BAT *b);
diff --git a/gdk/gdk_atoms.c b/gdk/gdk_atoms.c
--- a/gdk/gdk_atoms.c
+++ b/gdk/gdk_atoms.c
@@ -177,6 +177,8 @@ batUnfix(const bat *b)
* The incremental atom construction uses hardwired properties. This
* should be improved later on.
*/
+static MT_Lock GDKatomLock = MT_LOCK_INITIALIZER(GDKatomLock);
+
int
ATOMallocate(const char *id)
{
@@ -187,13 +189,13 @@ ATOMallocate(const char *id)
return int_nil;
}
- MT_lock_set(&GDKthreadLock);
+ MT_lock_set(&GDKatomLock);
t = ATOMindex(id);
if (t < 0) {
t = -t;
if (t == GDKatomcnt) {
if (GDKatomcnt == MAXATOMS) {
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
GDKerror("too many types");
return int_nil;
}
@@ -206,7 +208,7 @@ ATOMallocate(const char *id)
};
strcpy(BATatoms[t].name, id);
}
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
return t;
}
@@ -1925,11 +1927,11 @@ ATOMunknown_find(const char *nme)
int i, j = 0;
/* first try to find the atom */
- MT_lock_set(&GDKthreadLock);
+ MT_lock_set(&GDKatomLock);
for (i = 1; i < MAXATOMS; i++) {
if (unknown[i]) {
if (strcmp(unknown[i], nme) == 0) {
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
return -i;
}
} else if (j == 0)
@@ -1937,14 +1939,14 @@ ATOMunknown_find(const char *nme)
}
if (j == 0) {
/* no space for new atom (shouldn't happen) */
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
return 0;
}
if ((unknown[j] = GDKstrdup(nme)) == NULL) {
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
return 0;
}
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
return -j;
}
@@ -1962,7 +1964,7 @@ ATOMunknown_clean(void)
{
int i;
- MT_lock_set(&GDKthreadLock);
+ MT_lock_set(&GDKatomLock);
for (i = 1; i < MAXATOMS; i++) {
if(unknown[i]) {
GDKfree(unknown[i]);
@@ -1971,5 +1973,5 @@ ATOMunknown_clean(void)
break;
}
}
- MT_lock_unset(&GDKthreadLock);
+ MT_lock_unset(&GDKatomLock);
}
diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c
--- a/gdk/gdk_bbp.c
+++ b/gdk/gdk_bbp.c
@@ -2566,12 +2566,14 @@ BBPinsert(BAT *bn)
bn->batCacheid = i;
bn->creator_tid = MT_getpid();
+ MT_lock_set(&GDKswapLock(i));
BBP_status_set(i, BBPDELETING|BBPHOT);
BBP_cache(i) = NULL;
BBP_desc(i) = NULL;
BBP_refs(i) = 1; /* new bats have 1 pin */
BBP_lrefs(i) = 0; /* ie. no logical refs */
BBP_pid(i) = MT_getpid();
+ MT_lock_unset(&GDKswapLock(i));
#ifdef HAVE_HGE
if (bn->ttype == TYPE_hge)
diff --git a/gdk/gdk_private.h b/gdk/gdk_private.h
--- a/gdk/gdk_private.h
+++ b/gdk/gdk_private.h
@@ -421,7 +421,11 @@ ilog2(BUN x)
b && b->torderidx ? "O" : "", \
b ? b->timprints ? "I" : b->theap && b->theap->parentid &&
BBP_cache(b->theap->parentid) && BBP_cache(b->theap->parentid)->timprints ?
"(I)" : "" : ""
+#ifdef __SANITIZE_THREAD__
+#define BBP_BATMASK 31
+#else
#define BBP_BATMASK ((1 << (SIZEOF_SIZE_T + 5)) - 1)
+#endif
struct PROPrec {
enum prop_t id;
@@ -469,7 +473,6 @@ extern batlock_t GDKbatLock[BBP_BATMASK
extern size_t GDK_mmap_minsize_persistent; /* size after which we use memory
mapped files for persistent heaps */
extern size_t GDK_mmap_minsize_transient; /* size after which we use memory
mapped files for transient heaps */
extern size_t GDK_mmap_pagesize; /* mmap granularity */
-extern MT_Lock GDKthreadLock;
extern MT_Lock GDKtmLock;
#define BATcheck(tst, err) \
diff --git a/gdk/gdk_utils.c b/gdk/gdk_utils.c
--- a/gdk/gdk_utils.c
+++ b/gdk/gdk_utils.c
@@ -1203,6 +1203,8 @@ GDKprepareExit(void)
}
}
+static MT_Lock GDKthreadLock = MT_LOCK_INITIALIZER(GDKthreadLock);
+
void
GDKreset(int status)
{
@@ -1334,7 +1336,6 @@ GDKexit(int status)
*/
batlock_t GDKbatLock[BBP_BATMASK + 1];
-MT_Lock GDKthreadLock = MT_LOCK_INITIALIZER(GDKthreadLock);
/* GDKtmLock protects all accesses and changes to BAKDIR and SUBDIR */
MT_Lock GDKtmLock = MT_LOCK_INITIALIZER(GDKtmLock);
diff --git a/monetdb5/mal/mal.c b/monetdb5/mal/mal.c
--- a/monetdb5/mal/mal.c
+++ b/monetdb5/mal/mal.c
@@ -85,7 +85,6 @@ mal_init(char *modules[], int embedded)
#endif
initNamespace();
initParser();
- initHeartbeat();
err = malBootstrap(modules, embedded);
if (err != MAL_SUCCEED) {
@@ -98,6 +97,7 @@ mal_init(char *modules[], int embedded)
return -1;
}
initProfiler();
+ initHeartbeat();
return 0;
}
diff --git a/monetdb5/mal/mal_prelude.c b/monetdb5/mal/mal_prelude.c
--- a/monetdb5/mal/mal_prelude.c
+++ b/monetdb5/mal/mal_prelude.c
@@ -417,7 +417,7 @@ melFunction(bool command, const char *mo
}
static str
-malPrelude(Client c, int listing, int no_mapi_server)
+malPrelude(Client c, int listing)
{
int i;
str msg = MAL_SUCCEED;
@@ -443,8 +443,8 @@ malPrelude(Client c, int listing, int no
if (msg)
return msg;
- /* skip sql should be last to startup and mapi if
configured without mapi server */
- if (strcmp(mel_module[i].name, "sql") == 0 ||
(no_mapi_server && strcmp(mel_module[i].name, "mapi") == 0))
+ /* mapi should be last, and sql last before mapi */
+ if (strcmp(mel_module[i].name, "sql") == 0 ||
strcmp(mel_module[i].name, "mapi") == 0)
continue;
if (!mel_module[i].inits) {
msg = initModule(c, mel_module[i].name);
@@ -456,8 +456,8 @@ malPrelude(Client c, int listing, int no
msg = mel_module[i].inits();
if (msg)
return msg;
- /* skip sql should be last to startup and mapi if
configured without mapi server */
- if (strcmp(mel_module[i].name, "sql") == 0 ||
(no_mapi_server && strcmp(mel_module[i].name, "mapi") == 0))
+ /* mapi should be last, and sql last before mapi */
+ if (strcmp(mel_module[i].name, "sql") == 0 ||
strcmp(mel_module[i].name, "mapi") == 0)
continue;
msg = initModule(c, mel_module[i].name);
if (msg)
@@ -481,13 +481,15 @@ malIncludeModules(Client c, char *module
return msg;
}
/* load the mal code for these modules and execute preludes */
- if ((msg = malPrelude(c, listing, no_mapi_server)) != NULL)
+ if ((msg = malPrelude(c, listing)) != NULL)
+ return msg;
+ msg = initModule(c, "sql");
+ if (msg)
return msg;
- for(int i = 0; modules[i]; i++) {
- if (strcmp(modules[i], "sql") == 0) { /* start now */
- initModule(c, modules[i]);
- break;
- }
+ if (!no_mapi_server) {
+ msg = initModule(c, "mapi");
+ if (msg)
+ return msg;
}
return MAL_SUCCEED;
}
diff --git a/monetdb5/mal/mal_profiler.c b/monetdb5/mal/mal_profiler.c
--- a/monetdb5/mal/mal_profiler.c
+++ b/monetdb5/mal/mal_profiler.c
@@ -751,7 +751,8 @@ str
stopProfiler(Client cntxt)
{
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org
