[sgen] Fix race between complex descriptor allocation and complex object scanning

When we allocate the complex descriptor for a new type we might have the concurrent marker scan objects. While the concurrent marker scans the bitmap for a complex object, which resides in the complex_descriptor array, another thread might allocate a new complex descriptor, thus reallocate the complex_descriptor array, which would lead to corruption of the bitmap used by the concurrent marker.

We reuse the SgenArrayList which never reallocates, using instead a list of exponentially increasing arrays.

diff --git a/mono/sgen/sgen-array-list.c b/mono/sgen/sgen-array-list.c
index f84868e0a6f5f8d8999ce2ffd4e7947884ad9d3a..09ac01dfd5e8ee88f365d618b551d1b9a84c5be4 100644 (file)
--- a/mono/sgen/sgen-array-list.c
+++ b/mono/sgen/sgen-array-list.c
@@ -112,6 +112,32 @@ sgen_array_list_update_next_slot (SgenArrayList *array, guint32 new_index)
        }
 }
 
+/*
+ * Extension for the array list that allows fast allocation and index based fetching
+ * of long lived memory of various sizes, without the need of realloc. Not thread safe.
+ */
+guint32
+sgen_array_list_alloc_block (SgenArrayList *array, guint32 slots_to_add)
+{
+       guint32 new_index = array->next_slot;
+       guint32 old_capacity = array->capacity;
+
+       /* FIXME Don't allocate arrays that will be skipped */
+       /* There are no empty arrays between next_slot and capacity because we allocate incrementally */
+       while ((old_capacity - new_index) < slots_to_add) {
+               sgen_array_list_grow (array, old_capacity);
+               new_index = old_capacity;
+               old_capacity = array->capacity;
+       }
+
+       SGEN_ASSERT (0, sgen_array_list_index_bucket (new_index) == sgen_array_list_index_bucket (new_index + slots_to_add - 1),
+                       "We failed to allocate a continuous block of slots");
+
+       array->next_slot = new_index + slots_to_add;
+       /* The slot address will point to the allocated memory */
+       return new_index;
+}
+
 guint32
 sgen_array_list_add (SgenArrayList *array, gpointer ptr, int data, gboolean increase_size_before_set)
 {

diff --git a/mono/sgen/sgen-array-list.h b/mono/sgen/sgen-array-list.h
index fb37009a67230fdd0c9d328cf300cb78fae3ce12..5b9291693363250e7ab4916b887d6849d5397b65 100644 (file)
--- a/mono/sgen/sgen-array-list.h
+++ b/mono/sgen/sgen-array-list.h
@@ -129,6 +129,7 @@ sgen_array_list_get_slot (SgenArrayList *array, guint32 index)
 
 #define SGEN_ARRAY_LIST_END_FOREACH_SLOT_RANGE } }
 
+guint32 sgen_array_list_alloc_block (SgenArrayList *array, guint32 slots_to_add);
 guint32 sgen_array_list_add (SgenArrayList *array, gpointer ptr, int data, gboolean increase_size_before_set);
 guint32 sgen_array_list_find (SgenArrayList *array, gpointer ptr);
 void sgen_array_list_remove_nulls (SgenArrayList *array);

diff --git a/mono/sgen/sgen-descriptor.c b/mono/sgen/sgen-descriptor.c
index b30ab4334bc84338e9d65132da2b88df04911a43..248b07b8f50fff935da561c90488a52af682ac17 100644 (file)
--- a/mono/sgen/sgen-descriptor.c
+++ b/mono/sgen/sgen-descriptor.c
@@ -33,6 +33,7 @@
 
 #include "mono/sgen/sgen-gc.h"
 #include "mono/sgen/gc-internal-agnostic.h"
+#include "mono/sgen/sgen-array-list.h"
 
 #define MAX_USER_DESCRIPTORS 16
 
@@ -40,9 +41,7 @@
 #define ALIGN_TO(val,align) ((((guint64)val) + ((align) - 1)) & ~((align) - 1))
 
 
-static gsize* complex_descriptors = NULL;
-static int complex_descriptors_size = 0;
-static int complex_descriptors_next = 0;
+static SgenArrayList complex_descriptors = SGEN_ARRAY_LIST_INIT (NULL, NULL, NULL, INTERNAL_MEM_COMPLEX_DESCRIPTORS);
 static SgenUserRootMarkFunc user_descriptors [MAX_USER_DESCRIPTORS];
 static int user_descriptors_next = 0;
 static SgenDescriptor all_ref_root_descrs [32];
@@ -56,6 +55,8 @@ static int
 alloc_complex_descriptor (gsize *bitmap, int numbits)
 {
        int nwords, res, i;
+       volatile gpointer *slot;
+       gsize *descriptor;
 
        SGEN_ASSERT (0, sizeof (gsize) == sizeof (mword), "We expect gsize and mword to have same size");
 
@@ -63,38 +64,41 @@ alloc_complex_descriptor (gsize *bitmap, int numbits)
        nwords = numbits / GC_BITS_PER_WORD + 1;
 
        sgen_gc_lock ();
-       res = complex_descriptors_next;
        /* linear search, so we don't have duplicates with domain load/unload
         * this should not be performance critical or we'd have bigger issues
         * (the number and size of complex descriptors should be small).
         */
-       for (i = 0; i < complex_descriptors_next; ) {
-               if (complex_descriptors [i] == nwords) {
+       SGEN_ARRAY_LIST_FOREACH_SLOT (&complex_descriptors, slot) {
+               gsize first_word = *(gsize*)slot;
+               if (first_word == 0) {
+                       /* Unused slots should be 0 so we simply skip them */
+                       continue;
+               } else if (first_word == nwords) {
                        int j, found = TRUE;
                        for (j = 0; j < nwords - 1; ++j) {
-                               if (complex_descriptors [i + 1 + j] != bitmap [j]) {
+                               if (((gsize*)slot) [j + 1] != bitmap [j]) {
                                        found = FALSE;
                                        break;
                                }
                        }
                        if (found) {
                                sgen_gc_unlock ();
-                               return i;
+                               return __index;
                        }
                }
-               i += (int)complex_descriptors [i];
-       }
-       if (complex_descriptors_next + nwords > complex_descriptors_size) {
-               int new_size = complex_descriptors_size * 2 + nwords;
-               complex_descriptors = (gsize *)g_realloc (complex_descriptors, new_size * sizeof (gsize));
-               complex_descriptors_size = new_size;
-       }
-       SGEN_LOG (6, "Complex descriptor %d, size: %d (total desc memory: %d)", res, nwords, complex_descriptors_size);
-       complex_descriptors_next += nwords;
-       complex_descriptors [res] = nwords;
+               /* Skip the bitmap words */
+               __index += (guint32)(first_word - 1);
+               __offset += (guint32)(first_word - 1);
+       } SGEN_ARRAY_LIST_END_FOREACH_SLOT;
+
+       res = sgen_array_list_alloc_block (&complex_descriptors, nwords);
+
+       SGEN_LOG (6, "Complex descriptor %d, size: %d (total desc memory: %d)", res, nwords, complex_descriptors.capacity);
+       descriptor = (gsize*)sgen_array_list_get_slot (&complex_descriptors, res);
+       descriptor [0] = nwords;
        for (i = 0; i < nwords - 1; ++i) {
-               complex_descriptors [res + 1 + i] = bitmap [i];
-               SGEN_LOG (6, "\tvalue: %p", (void*)complex_descriptors [res + 1 + i]);
+               descriptor [1 + i] = bitmap [i];
+               SGEN_LOG (6, "\tvalue: %p", (void*)descriptor [1 + i]);
        }
        sgen_gc_unlock ();
        return res;
@@ -103,7 +107,7 @@ alloc_complex_descriptor (gsize *bitmap, int numbits)
 gsize*
 sgen_get_complex_descriptor (SgenDescriptor desc)
 {
-       return complex_descriptors + (desc >> LOW_TYPE_BITS);
+       return (gsize*) sgen_array_list_get_slot (&complex_descriptors, desc >> LOW_TYPE_BITS);
 }
 
 /*
@@ -313,7 +317,7 @@ sgen_make_user_root_descriptor (SgenUserRootMarkFunc marker)
 void*
 sgen_get_complex_descriptor_bitmap (SgenDescriptor desc)
 {
-       return complex_descriptors + (desc >> ROOT_DESC_TYPE_SHIFT);
+       return (void*) sgen_array_list_get_slot (&complex_descriptors, desc >> ROOT_DESC_TYPE_SHIFT);
 }
 
 SgenUserRootMarkFunc

diff --git a/mono/sgen/sgen-gc.h b/mono/sgen/sgen-gc.h
index 3f61780452162b286a6056be27a731aa4e8575ff..94b453b6e279352ad5be9405919caa66257513a9 100644 (file)
--- a/mono/sgen/sgen-gc.h
+++ b/mono/sgen/sgen-gc.h
@@ -340,6 +340,7 @@ enum {
        INTERNAL_MEM_BINARY_PROTOCOL,
        INTERNAL_MEM_TEMPORARY,
        INTERNAL_MEM_LOG_ENTRY,
+       INTERNAL_MEM_COMPLEX_DESCRIPTORS,
        INTERNAL_MEM_FIRST_CLIENT
 };
 

diff --git a/mono/sgen/sgen-internal.c b/mono/sgen/sgen-internal.c
index 5229596e6d48903ed849a7f69defc03fa9b4e230..ccff562737ab4deac9515b83c266f95147ebf36d 100644 (file)
--- a/mono/sgen/sgen-internal.c
+++ b/mono/sgen/sgen-internal.c
@@ -140,6 +140,7 @@ description_for_type (int type)
        case INTERNAL_MEM_BINARY_PROTOCOL: return "binary-protocol";
        case INTERNAL_MEM_TEMPORARY: return "temporary";
        case INTERNAL_MEM_LOG_ENTRY: return "log-entry";
+       case INTERNAL_MEM_COMPLEX_DESCRIPTORS: return "complex-descriptors";
        default: {
                const char *description = sgen_client_description_for_internal_mem_type (type);
                SGEN_ASSERT (0, description, "Unknown internal mem type");