Skip to content

Commit 38a1510

Browse files
ShadauxCat0xFA11
ShadauxCat
and
0xFA11
authored
fix!: NativeList<T> causes crashes when used in RPCs [NCCBUG-106] (#1901)
* WIP: fixing crashes when passing a NativeList to an RPC * continued WIP, not ready for anything. * Finally got it working correctly, still need to write tests. * Added tests for templated types, fixed some issues around the ILPP generated for them. * - Fixed failing tests (FixedString is not natively supported anymore without a ForceSerializeByMemcpy wrapper due to the ISerializeByMemcpy tag requirement) - Made sure there's at least one test exercising Unity types going through RPCs... more tests exercising all of these are needed later. * Removed some debug logging. * Standards * Changelog * Fix testproject-tools-integration * Slight refactor to let ILPP find more types it needs to generate initializers for by looking for instances of NetworkVariable<T> and NetworkList<T>. This was required because runtime reflection doesn't work in AoT consoles; the runtime reflection will now only ever be needed if someone does something particularly odd like, for some reason, making a NetworkVariable or NetworkList that's not a field of a NetworkBehaviour. Made an class to go between NetworkVariable and NetworkVariableBase so that future uses for these serialization functions will also be picked up automatically by ILPP. Also, review feedback: - Renamed ISerializeByMemcpy to INetworkSerializeByMemcpy - Reverted Burst json file - Removed an errant empty line * Update com.unity.netcode.gameobjects/CHANGELOG.md Co-authored-by: Fatih Mar <[email protected]> * Standards. * super minor touch * xmldocs and comments Co-authored-by: Fatih Mar <[email protected]>
1 parent 5c5566e commit 38a1510

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+1829
-685
lines changed

‎com.unity.netcode.gameobjects/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
1111

1212
### Changed
1313

14+
- `unmanaged` structs are no longer universally accepted as RPC parameters because some structs (i.e., structs with pointers in them, such as `NativeList<T>`) can't be supported by the default memcpy struct serializer. Structs that are intended to be serialized across the network must add `INetworkSerializeByMemcpy` to the interface list (i.e., `struct Foo : INetworkSerializeByMemcpy`). This interface is empty and just serves to mark the struct as compatible with memcpy serialization. For external structs you can't edit, you can pass them to RPCs by wrapping them in `ForceNetworkSerializeByMemcpy<T>`. (#1901)
15+
1416
### Removed
1517

1618
### Fixed
@@ -19,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
1921
- Fixed client throwing an exception if it has messages in the outbound queue when processing the `NetworkEvent.Disconnect` event and is using UTP. (#1884)
2022
- Fixed issue during client synchronization if 'ValidateSceneBeforeLoading' returned false it would halt the client synchronization process resulting in a client that was approved but not synchronized or fully connected with the server. (#1883)
2123
- Fixed an issue where UNetTransport.StartServer would return success even if the underlying transport failed to start (#854)
24+
- Passing generic types to RPCs no longer causes a native crash (#1901)
2225

2326
## [Unreleased]
2427

‎com.unity.netcode.gameobjects/Editor/CodeGen/CodeGenHelpers.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ internal static class CodeGenHelpers
2727
public static readonly string ServerRpcSendParams_FullName = typeof(ServerRpcSendParams).FullName;
2828
public static readonly string ServerRpcReceiveParams_FullName = typeof(ServerRpcReceiveParams).FullName;
2929
public static readonly string INetworkSerializable_FullName = typeof(INetworkSerializable).FullName;
30+
public static readonly string INetworkSerializeByMemcpy_FullName = typeof(INetworkSerializeByMemcpy).FullName;
3031
public static readonly string UnityColor_FullName = typeof(Color).FullName;
3132
public static readonly string UnityColor32_FullName = typeof(Color32).FullName;
3233
public static readonly string UnityVector2_FullName = typeof(Vector2).FullName;
@@ -77,6 +78,35 @@ public static bool IsSubclassOf(this TypeDefinition typeDefinition, string class
7778
return false;
7879
}
7980

81+
public static string FullNameWithGenericParameters(this TypeReference typeReference, GenericParameter[] contextGenericParameters, TypeReference[] contextGenericParameterTypes)
82+
{
83+
var name = typeReference.FullName;
84+
if (typeReference.HasGenericParameters)
85+
{
86+
name += "<";
87+
for (var i = 0; i < typeReference.Resolve().GenericParameters.Count; ++i)
88+
{
89+
if (i != 0)
90+
{
91+
name += ", ";
92+
}
93+
94+
for (var j = 0; j < contextGenericParameters.Length; ++j)
95+
{
96+
if (typeReference.GenericParameters[i].FullName == contextGenericParameters[i].FullName)
97+
{
98+
name += contextGenericParameterTypes[i].FullName;
99+
break;
100+
}
101+
}
102+
}
103+
104+
name += ">";
105+
}
106+
107+
return name;
108+
}
109+
80110
public static bool HasInterface(this TypeReference typeReference, string interfaceTypeFullName)
81111
{
82112
if (typeReference.IsArray)

‎com.unity.netcode.gameobjects/Editor/CodeGen/INetworkSerializableILPP.cs

Lines changed: 170 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,37 @@ public override bool WillProcess(ICompiledAssembly compiledAssembly) =>
2424

2525
private readonly List<DiagnosticMessage> m_Diagnostics = new List<DiagnosticMessage>();
2626

27+
private TypeReference ResolveGenericType(TypeReference type, List<TypeReference> typeStack)
28+
{
29+
var genericName = type.Name;
30+
var lastType = (GenericInstanceType)typeStack[typeStack.Count - 1];
31+
var resolvedType = lastType.Resolve();
32+
typeStack.RemoveAt(typeStack.Count - 1);
33+
for (var i = 0; i < resolvedType.GenericParameters.Count; ++i)
34+
{
35+
var parameter = resolvedType.GenericParameters[i];
36+
if (parameter.Name == genericName)
37+
{
38+
var underlyingType = lastType.GenericArguments[i];
39+
if (underlyingType.Resolve() == null)
40+
{
41+
return ResolveGenericType(underlyingType, typeStack);
42+
}
43+
44+
return underlyingType;
45+
}
46+
}
47+
48+
return null;
49+
}
50+
2751
public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
2852
{
2953
if (!WillProcess(compiledAssembly))
3054
{
3155
return null;
3256
}
3357

34-
3558
m_Diagnostics.Clear();
3659

3760
// read
@@ -50,16 +73,124 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
5073
{
5174
if (ImportReferences(mainModule))
5275
{
53-
var types = mainModule.GetTypes()
54-
.Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializable_FullName) && !t.Resolve().IsAbstract && t.Resolve().IsValueType)
76+
// Initialize all the delegates for various NetworkVariable types to ensure they can be serailized
77+
78+
// Find all types we know we're going to want to serialize.
79+
// The list of these types includes:
80+
// - Non-generic INetworkSerializable types
81+
// - Non-Generic INetworkSerializeByMemcpy types
82+
// - Enums that are not declared within generic types
83+
// We can't process generic types because, to initialize a generic, we need a value
84+
// for `T` to initialize it with.
85+
var networkSerializableTypes = mainModule.GetTypes()
86+
.Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializable_FullName) && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
87+
.ToList();
88+
var structTypes = mainModule.GetTypes()
89+
.Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName) && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
90+
.ToList();
91+
var enumTypes = mainModule.GetTypes()
92+
.Where(t => t.Resolve().IsEnum && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
5593
.ToList();
56-
// process `INetworkMessage` types
57-
if (types.Count == 0)
94+
95+
// Now, to support generics, we have to do an extra pass.
96+
// We look for any type that's a NetworkBehaviour type
97+
// Then we look through all the fields in that type, finding any field whose type is
98+
// descended from `NetworkVariableSerialization`. Then we check `NetworkVariableSerialization`'s
99+
// `T` value, and if it's a generic, then we know it was missed in the above sweep and needs
100+
// to be initialized. Now we have a full generic instance rather than a generic definition,
101+
// so we can validly generate an initializer for that particular instance of the generic type.
102+
var networkSerializableTypesSet = new HashSet<TypeReference>(networkSerializableTypes);
103+
var structTypesSet = new HashSet<TypeReference>(structTypes);
104+
var enumTypesSet = new HashSet<TypeReference>(enumTypes);
105+
var typeStack = new List<TypeReference>();
106+
foreach (var type in mainModule.GetTypes())
107+
{
108+
// Check if it's a NetworkBehaviour
109+
if (type.IsSubclassOf(CodeGenHelpers.NetworkBehaviour_FullName))
110+
{
111+
// Iterate fields looking for NetworkVariableSerialization fields
112+
foreach (var field in type.Fields)
113+
{
114+
// Get the field type and its base type
115+
var fieldType = field.FieldType;
116+
var baseType = fieldType.Resolve().BaseType;
117+
// This type stack is used for resolving NetworkVariableSerialization's T value
118+
// When looking at base types, we get the type definition rather than the
119+
// type reference... which means that we get the generic definition with an
120+
// undefined T rather than the instance with the type filled in.
121+
// We then have to walk backward back down the type stack to resolve what T
122+
// is.
123+
typeStack.Clear();
124+
typeStack.Add(fieldType);
125+
// Iterate through the base types until we get to Object.
126+
// Object is the base for everything so we'll stop when we hit that.
127+
while (baseType.Name != mainModule.TypeSystem.Object.Name)
128+
{
129+
// If we've found a NetworkVariableSerialization type...
130+
if (baseType.IsGenericInstance && baseType.Resolve() == m_NetworkVariableSerializationType)
131+
{
132+
// Then we need to figure out what T is
133+
var genericType = (GenericInstanceType)baseType;
134+
var underlyingType = genericType.GenericArguments[0];
135+
if (underlyingType.Resolve() == null)
136+
{
137+
underlyingType = ResolveGenericType(underlyingType, typeStack);
138+
}
139+
140+
// If T is generic...
141+
if (underlyingType.IsGenericInstance)
142+
{
143+
// Then we pick the correct set to add it to and set it up
144+
// for initialization.
145+
if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
146+
{
147+
networkSerializableTypesSet.Add(underlyingType);
148+
}
149+
150+
if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
151+
{
152+
structTypesSet.Add(underlyingType);
153+
}
154+
155+
if (underlyingType.Resolve().IsEnum)
156+
{
157+
enumTypesSet.Add(underlyingType);
158+
}
159+
}
160+
161+
break;
162+
}
163+
164+
typeStack.Add(baseType);
165+
baseType = baseType.Resolve().BaseType;
166+
}
167+
}
168+
}
169+
// We'll also avoid some confusion by ensuring users only choose one of the two
170+
// serialization schemes - by method OR by memcpy, not both. We'll also do a cursory
171+
// check that INetworkSerializeByMemcpy types are unmanaged.
172+
else if (type.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
173+
{
174+
if (type.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
175+
{
176+
m_Diagnostics.AddError($"{nameof(INetworkSerializeByMemcpy)} types may not implement {nameof(INetworkSerializable)} - choose one or the other.");
177+
}
178+
if (!type.IsValueType)
179+
{
180+
m_Diagnostics.AddError($"{nameof(INetworkSerializeByMemcpy)} types must be unmanaged types.");
181+
}
182+
}
183+
}
184+
185+
if (networkSerializableTypes.Count + structTypes.Count + enumTypes.Count == 0)
58186
{
59187
return null;
60188
}
61189

62-
CreateModuleInitializer(assemblyDefinition, types);
190+
// Finally we add to the module initializer some code to initialize the delegates in
191+
// NetworkVariableSerialization<T> for all necessary values of T, by calling initialization
192+
// methods in NetworkVariableHelpers.
193+
CreateModuleInitializer(assemblyDefinition, networkSerializableTypesSet.ToList(), structTypesSet.ToList(), enumTypesSet.ToList());
63194
}
64195
else
65196
{
@@ -94,9 +225,15 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
94225
return new ILPostProcessResult(new InMemoryAssembly(pe.ToArray(), pdb.ToArray()), m_Diagnostics);
95226
}
96227

97-
private MethodReference m_InitializeDelegates_MethodRef;
228+
private MethodReference m_InitializeDelegatesNetworkSerializable_MethodRef;
229+
private MethodReference m_InitializeDelegatesStruct_MethodRef;
230+
private MethodReference m_InitializeDelegatesEnum_MethodRef;
98231

99-
private const string k_InitializeMethodName = nameof(NetworkVariableHelper.InitializeDelegates);
232+
private TypeDefinition m_NetworkVariableSerializationType;
233+
234+
private const string k_InitializeNetworkSerializableMethodName = nameof(NetworkVariableHelper.InitializeDelegatesNetworkSerializable);
235+
private const string k_InitializeStructMethodName = nameof(NetworkVariableHelper.InitializeDelegatesStruct);
236+
private const string k_InitializeEnumMethodName = nameof(NetworkVariableHelper.InitializeDelegatesEnum);
100237

101238
private bool ImportReferences(ModuleDefinition moduleDefinition)
102239
{
@@ -106,11 +243,18 @@ private bool ImportReferences(ModuleDefinition moduleDefinition)
106243
{
107244
switch (methodInfo.Name)
108245
{
109-
case k_InitializeMethodName:
110-
m_InitializeDelegates_MethodRef = moduleDefinition.ImportReference(methodInfo);
246+
case k_InitializeNetworkSerializableMethodName:
247+
m_InitializeDelegatesNetworkSerializable_MethodRef = moduleDefinition.ImportReference(methodInfo);
248+
break;
249+
case k_InitializeStructMethodName:
250+
m_InitializeDelegatesStruct_MethodRef = moduleDefinition.ImportReference(methodInfo);
251+
break;
252+
case k_InitializeEnumMethodName:
253+
m_InitializeDelegatesEnum_MethodRef = moduleDefinition.ImportReference(methodInfo);
111254
break;
112255
}
113256
}
257+
m_NetworkVariableSerializationType = moduleDefinition.ImportReference(typeof(NetworkVariableSerialization<>)).Resolve();
114258
return true;
115259
}
116260

@@ -139,7 +283,7 @@ private MethodDefinition GetOrCreateStaticConstructor(TypeDefinition typeDefinit
139283
// C# (that attribute doesn't exist in Unity, but the static module constructor still works)
140284
// https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.moduleinitializerattribute?view=net-5.0
141285
// https://web.archive.org/web/20100212140402/http://blogs.msdn.com/junfeng/archive/2005/11/19/494914.aspx
142-
private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeDefinition> networkSerializableTypes)
286+
private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeReference> networkSerializableTypes, List<TypeReference> structTypes, List<TypeReference> enumTypes)
143287
{
144288
foreach (var typeDefinition in assembly.MainModule.Types)
145289
{
@@ -151,9 +295,23 @@ private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeDefin
151295

152296
var instructions = new List<Instruction>();
153297

298+
foreach (var type in structTypes)
299+
{
300+
var method = new GenericInstanceMethod(m_InitializeDelegatesStruct_MethodRef);
301+
method.GenericArguments.Add(type);
302+
instructions.Add(processor.Create(OpCodes.Call, method));
303+
}
304+
154305
foreach (var type in networkSerializableTypes)
155306
{
156-
var method = new GenericInstanceMethod(m_InitializeDelegates_MethodRef);
307+
var method = new GenericInstanceMethod(m_InitializeDelegatesNetworkSerializable_MethodRef);
308+
method.GenericArguments.Add(type);
309+
instructions.Add(processor.Create(OpCodes.Call, method));
310+
}
311+
312+
foreach (var type in enumTypes)
313+
{
314+
var method = new GenericInstanceMethod(m_InitializeDelegatesEnum_MethodRef);
157315
method.GenericArguments.Add(type);
158316
instructions.Add(processor.Create(OpCodes.Call, method));
159317
}

0 commit comments

Comments
 (0)