GC Clustering does not mark mutable objects as reachable

Development Platform & Builds
, ,
1

Hi All,

We have tracked down what we think is a legitimate bug in MarkReferencedClustersAsReachable(). It seems like this will not mark the mutable objects as reachable. This can end up leaving the mutable objects marked as unreachable and thus incorrectly purged if nothing else keeps a reference to it (see my repro steps).

We think the correct code should be something like as follows:

static FORCENOINLINE void MarkReferencedClustersAsReachable(int32 ClusterIndex, ContainerType& ObjectsToSerialize)
{
	/* -CUT- for brevity */

	for (int32& ReferncedClusterIndex : Cluster.ReferencedClusters)
	{
		if (ReferncedClusterIndex >= 0) // Garbag Elimination support
		{
			FUObjectItem* ReferencedClusterRootObjectItem = GUObjectArray.IndexToObjectUnsafeForGC(ReferncedClusterIndex);
			if (!ReferencedClusterRootObjectItem->HasAnyFlags(EInternalObjectFlags::Garbage))
			{
				// @CHANGE: BEGIN - Fix for clustering
				// Originally:
				//   FGCFlags::FastMarkAsReachableInterlocked_ForGC(ReferencedClusterRootObjectItem);
				// But this fails to mark mutable links/attached clusters properly, leading to verification failures.
				// Copying how it treats cluster links in MarkClusterMutableObjectsAsReachable gives:
				if (FGCFlags::MarkAsReachableInterlocked_ForGC(ReferencedClusterRootObjectItem))
				{	// If we are first, then recursively mark:
					MarkReferencedClustersAsReachable<Options>(ReferencedClusterRootObjectItem->GetClusterIndex(), ObjectsToSerialize);
				}
				// @CHANGE: END - Fix for clustering
			}
			else
			{
				// Garbage Elimination support for clusters
				ReferncedClusterIndex = -1;
				bAddClusterObjectsToSerialize = true;
			}
		}
	}
	/* -CUT- for brevity */
}

Without this change we could end up in situations where the cluster was the only thing referencing the object and it incorrectly ended up purging the object despite still being referenced by another object within the cluster itself.

Does this change look reasonable? It seems to fix the issue for us but we’re not sure of the full ramifications of this change.

Cheers!

-Steven

[Attachment Removed]

2

Steps to Reproduce

  1. Enable GC asset clustering (gc.AssetClustereringEnabled 1), in UObjectClusters.cpp (p.s. this a typo)
  2. Have an actor (actor A) with a TObjectPtr<> to another actor which is owned by a different owner, but is the sole referencer keeping the referenced object alive.
  3. A cluster will attempt to be created for Actor A which includes the referenced object as part of the mutable objects (FUObjectCluster::MutableObjects)
  4. Perform a GC. Actor A will be kept alive, but the parallel reachability does not mark it’s child MutableObjects as reachable. The object will be purged upon the next GC.
    [Attachment Removed]
3

Great find, Steven! In fact, I’m going to make the same change on our end. Thank you for reporting this!

[Attachment Removed]