[mono][sgen] Make color visible to client permanent

src/coreclr/gc/gcbridge.cpp

@@ -186,6 +186,11 @@ struct ColorData
     // Count of colors that list this color in their otherColors
     unsigned incomingColors : INCOMING_COLORS_BITS;
     unsigned visited : 1;
+    // ColorVisibleToClient for a ColorData* can change over the course of bridge processing which
+    // is problematic. We fix this by setting this flag when a color is detected as visible to client.
+    // Once the flag is set, the color is pinned to being visible to client, even though it could lose
+    // some xrefs, making it not satisfy the BridgelessColorIsHeavy condition.
+    unsigned visibleToClient : 1;
 };
 
 // Represents one managed object. Equivalent of new/old bridge "HashEntry"
@@ -233,7 +238,19 @@ static bool BridgelessColorIsHeavy(ColorData* data)
 // Should color be made visible to client?
 static bool ColorVisibleToClient(ColorData* data)
 {
-    return DynPtrArraySize(&data->bridges) || BridgelessColorIsHeavy(data);
+    if (data->visibleToClient)
+        return true;
+
+    if (DynPtrArraySize(&data->bridges) || BridgelessColorIsHeavy(data))
+    {
+        data->visibleToClient = true;
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+
 }
 
 // Stacks of ScanData objects used for tarjan algorithm.
@@ -1186,9 +1203,9 @@ static MarkCrossReferencesArgs* BuildSccCallbackData()
         ColorData* cd;
         for (cd = &cur->data[0]; cd < cur->nextData; cd++)
         {
-            int bridges = DynPtrArraySize(&cd->bridges);
-            if (!(bridges || BridgelessColorIsHeavy(cd)))
+            if (!ColorVisibleToClient(cd))
                 continue;
+            int bridges = DynPtrArraySize(&cd->bridges);
 
             apiSccs[apiIndex].Count = bridges;
             uintptr_t *contexts = new (nothrow) uintptr_t[bridges];

src/mono/mono/metadata/sgen-tarjan-bridge.c

@@ -96,6 +96,11 @@ typedef struct {
 	// Count of colors that list this color in their other_colors
 	unsigned incoming_colors : INCOMING_COLORS_BITS;
 	unsigned visited : 1;
+	// color_visible_to_client for a ColorData* can change over the course of bridge processing which
+	// is problematic. We fix this by setting this flag when a color is detected as visible to client.
+	// Once the flag is set, the color is pinned to being visible to client, even though it could lose
+	// some xrefs, making it not satisfy the bridgeless_color_is_heavy condition.
+	unsigned visible_to_client : 1;
 } ColorData;
 
 // Represents one managed object. Equivalent of new/old bridge "HashEntry"
@@ -140,8 +145,17 @@ bridgeless_color_is_heavy (ColorData *data) {
 
 // Should color be made visible to client?
 static gboolean
-color_visible_to_client (ColorData *data) {
-	return dyn_array_ptr_size (&data->bridges) || bridgeless_color_is_heavy (data);
+color_visible_to_client (ColorData *data)
+{
+	if (data->visible_to_client)
+		return TRUE;
+
+	if (dyn_array_ptr_size (&data->bridges) || bridgeless_color_is_heavy (data)) {
+		data->visible_to_client = TRUE;
+		return TRUE;
+	} else {
+		return FALSE;
+	}
 }
 
 // Stacks of ScanData objects used for tarjan algorithm.
@@ -1105,9 +1119,9 @@ processing_build_callback_data (int generation)
 	for (cur = root_color_bucket; cur; cur = cur->next) {
 		ColorData *cd;
 		for (cd = &cur->data [0]; cd < cur->next_data; ++cd) {
-			int bridges = dyn_array_ptr_size (&cd->bridges);
-			if (!(bridges || bridgeless_color_is_heavy (cd)))
+			if (!color_visible_to_client (cd))
 				continue;
+			int bridges = dyn_array_ptr_size (&cd->bridges);
 
 			api_sccs [api_index] = (MonoGCBridgeSCC *)sgen_alloc_internal_dynamic (sizeof (MonoGCBridgeSCC) + sizeof (MonoObject*) * bridges, INTERNAL_MEM_BRIDGE_DATA, TRUE);
 			api_sccs [api_index]->is_alive = FALSE;

src/tests/GC/Features/Bridge/Bridge.cs

@@ -89,6 +89,12 @@ public class Bridge1 : BridgeBase
     }
 }
 
+[InlineArray(14)]
+public struct InlineData
+{
+    public object obj;
+}
+
 // 128 size
 public class Bridge14 : BridgeBase
 {
@@ -107,7 +113,7 @@ public class NonBridge2 : NonBridge
 
 public class NonBridge14
 {
-    public object a,b,c,d,e,f,g,h,i,j,k,l,m,n;
+    public InlineData Data;
 }
 
 
@@ -403,6 +409,27 @@ static void SetupFragmentation<TBridge, TNonBridge>()
         }
     }
 
+    public static void BridgelessHeavyColorChanging()
+    {
+        Bridge1[] left = new Bridge1[8];
+        for (int i = 0; i < 8; i++)
+            left[i] = new Bridge1();
+        Bridge[] right = new Bridge[7];
+        for (int i = 0; i < 7; i++)
+            right[i] = new Bridge();
+        NonBridge2 right7 = new NonBridge2();
+
+        NonBridge14 mid = new NonBridge14();
+
+        for (int i = 0; i < 8; i++)
+            left[i].Link = mid;
+        for (int i = 0; i < 7; i++)
+            mid.Data[i] = right[i];
+        mid.Data[7] = right7;
+        right7.Link = right[6];
+        right7.Link2 = right[5];
+    }
+
     public static int Main(string[] args)
     {
         TestLinkedList();
@@ -417,6 +444,7 @@ public static int Main(string[] args)
         RunGraphTest(NestedCycles);
         RunGraphTest(FauxHeavyNodeWithCycles);
         RunGraphTest(Spider);
+        RunGraphTest(BridgelessHeavyColorChanging);
         return 100;
     }
 }