MonetDB: Dec2023 - Fixed a data race in strimps creation.

[Sjoerd Mullender via checkin-list]

Changeset: f5f2a0f6507e for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/f5f2a0f6507e
Modified Files:
        gdk/gdk_strimps.c
Branch: Dec2023
Log Message:

Fixed a data race in strimps creation.


diffs (82 lines):

diff --git a/gdk/gdk_strimps.c b/gdk/gdk_strimps.c
--- a/gdk/gdk_strimps.c
+++ b/gdk/gdk_strimps.c
@@ -838,10 +838,10 @@ BATsetstrimps(BAT *b)
  * in the BAT.
  */
 #define STRIMP_COMPLETE(b)                                             \
-       (b)->tstrimps != NULL &&                                        \
-               (b)->tstrimps != (Strimps *)1 &&                        \
-               (b)->tstrimps != (Strimps *)2 &&                        \
-               (((b)->tstrimps->strimps.free - ((char 
*)(b)->tstrimps->bitstrings_base - (b)->tstrimps->strimps.base)) == 
(b)->batCount*sizeof(uint64_t))
+       ((b)->tstrimps != NULL &&                                       \
+        (b)->tstrimps != (Strimps *)1 &&                               \
+        (b)->tstrimps != (Strimps *)2 &&                               \
+        (((b)->tstrimps->strimps.free - ((char 
*)(b)->tstrimps->bitstrings_base - (b)->tstrimps->strimps.base)) == 
(b)->batCount*sizeof(uint64_t)))
 
 
 /* Strimp creation.
@@ -886,16 +886,17 @@ STRMPcreate(BAT *b, BAT *s)
                pb = b;
        }
 
+       /* First thread to take the lock will read the strimp from disk
+        * or construct the strimp header.
+        */
+       MT_lock_set(&pb->batIdxLock);
+
        /* Strimp creation was requested. There are three cases:
         *  - The strimp is on disk (pb->tstrimps == 1)
         *  - The strimp needs to be created (pb->tstrimps == 2)
         *  - Finally the pointer might have been changed to NULL in another 
thread.
         */
        if (pb->tstrimps == NULL || pb->tstrimps == (Strimps*)1 || pb->tstrimps 
== (Strimps*)2) {
-               /* First thread to take the lock will read the strimp
-                * from disk or construct the strimp header.
-                */
-               MT_lock_set(&pb->batIdxLock);
                /* The strimp needs to be created. The rest of the
                 * creation code assumes that the pointer to the strimps
                 * is NULL. Make it so.
@@ -933,10 +934,10 @@ STRMPcreate(BAT *b, BAT *s)
                        }
                        pb->tstrimps = r;
                }
-               MT_lock_unset(&pb->batIdxLock);
        }
 
        if (STRIMP_COMPLETE(pb)) {
+               MT_lock_unset(&pb->batIdxLock);
                if (pb != b)
                        BBPunfix(pb->batCacheid);
                return GDK_SUCCEED;
@@ -947,6 +948,10 @@ STRMPcreate(BAT *b, BAT *s)
        MT_thread_setalgorithm("create strimp index");
        r = pb->tstrimps;
        STRMPincref(r);
+       if (pb != b) {
+               MT_lock_unset(&pb->batIdxLock);
+               MT_lock_set(&b->batIdxLock);
+       }
        dh = (uint64_t *)r->bitstrings_base + b->hseqbase;
        canditer_init(&ci, b, s);
 
@@ -961,13 +966,16 @@ STRMPcreate(BAT *b, BAT *s)
        }
        bat_iterator_end(&bi);
 
-       MT_lock_set(&b->batIdxLock);
+       if (pb != b) {
+               MT_lock_unset(&b->batIdxLock);
+               MT_lock_set(&pb->batIdxLock);
+       }
        r->strimps.free += b->batCount*sizeof(uint64_t);
        /* The thread that reaches this point last needs to write the strimp to 
disk. */
        if ((r->strimps.free - ((char *)r->bitstrings_base - r->strimps.base)) 
== b->batCount*sizeof(uint64_t)) {
                persistStrimp(pb);
        }
-       MT_lock_unset(&b->batIdxLock);
+       MT_lock_unset(&pb->batIdxLock);
        STRMPdecref(r, false);
 
        TRC_DEBUG(ACCELERATOR, "strimp creation took " LLFMT " usec\n", 
GDKusec()-t0);
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org