-
-
Notifications
You must be signed in to change notification settings - Fork 49
Fix: Prevent infinite loop when serializing assets with reflection types #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix: Prevent infinite loop when serializing assets with reflection types #296
Conversation
Filter out System.Reflection types (RuntimeType, RuntimeModule, RuntimeAssembly) from serialization to prevent circular references causing OutOfMemoryException when reading assets with AssetReferences. Reflection metadata cannot be meaningfully serialized to JSON or reconstructed, so excluding it improves both stability and output quality for read/write operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an infinite recursion bug in the Unity MCP Plugin's serialization system that occurred when serializing Unity assets containing System.Reflection types. The fix prevents OutOfMemoryException crashes by filtering out reflection metadata types (RuntimeType, RuntimeModule, RuntimeAssembly, etc.) from serialization, as these types form circular reference chains and provide no useful information for AI analysis.
Key changes:
- Added
IsReflectionType()helper method to identify and filter System.Reflection namespace types - Applied reflection type filtering to both field and property serialization in converter classes
- Added null-check guard when adding components to prevent silent failures
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| UnityGenericReflectionConvertor.cs | Implements reflection type filtering in GetSerializableFields() and GetSerializableProperties() with new IsReflectionType() helper |
| UnityArrayReflectionConvertor.cs | Applies identical reflection type filtering logic to array converter |
| GameObject.AddComponent.cs | Adds null-check validation after AddComponent() to warn about failures |
...-MCP-Plugin/Assets/root/Runtime/ReflectionConverters/Base/UnityGenericReflectionConvertor.cs
Outdated
Show resolved
Hide resolved
...-MCP-Plugin/Assets/root/Runtime/ReflectionConverters/Base/UnityGenericReflectionConvertor.cs
Outdated
Show resolved
Hide resolved
|
I've addressed feedback issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Catch System.Reflection.* namespace types | ||
| // Note: System.RuntimeType and System.Type are in System namespace, not System.Reflection | ||
| return fullName.StartsWith("System.Reflection.") || | ||
| fullName == "System.RuntimeType" || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it to type comparison instead of string comparison
type == typeof(System.RuntimeType)
type == typeof(System.Type)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.RuntimeType is not a real type
|
|
||
| // Catch System.Reflection.* namespace types | ||
| // Note: System.RuntimeType and System.Type are in System namespace, not System.Reflection | ||
| return fullName.StartsWith("System.Reflection.") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure sure about ignoring entire namespace System.Reflection.*. Any purposes of this?
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null) | ||
| .Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType)); | ||
|
|
||
| public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| { | ||
| var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger); | ||
| if (baseProperties == null) return null; | ||
|
|
||
| return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type.
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null) | ||
| .Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType)); | ||
|
|
||
| public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| { | ||
| var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger); | ||
| if (baseProperties == null) return null; | ||
|
|
||
| return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type.
Problem
Serializing
AssetReferenceSpritecaused infinite recursion:RuntimeType → RuntimeModule → RuntimeAssembly → RuntimeModule → ...
Result: OutOfMemoryException, massive log spam, crash.
Solution
Filter System.Reflection types from
GetSerializableFields()andGetSerializableProperties():Why Correct
Reflection metadata:
Impact
✅ Fixes OutOfMemoryException crashes
✅ Cleaner JSON output without metadata noise
✅ No breaking changes - filters unsupported types only