-
Notifications
You must be signed in to change notification settings - Fork 106
feat: improve garbage collection for injected types & implement finalizer support & stop leaking memory where we can avoid it. #216
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: master
Are you sure you want to change the base?
Changes from all commits
6f43413
4d87b31
d905c92
a29f0f6
4a4ce64
2e48c74
bba83b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,10 @@ | |
| using System.Reflection.Emit; | ||
| using System.Runtime.InteropServices; | ||
| using System.Text; | ||
| using Il2CppInterop.Common; | ||
| using Il2CppInterop.Runtime.Injection; | ||
| using Il2CppInterop.Runtime.InteropTypes; | ||
| using Il2CppInterop.Runtime.InteropTypes.Fields; | ||
| using Il2CppInterop.Runtime.Runtime; | ||
| using Microsoft.Extensions.Logging; | ||
| using Object = Il2CppSystem.Object; | ||
| using ValueType = Il2CppSystem.ValueType; | ||
|
|
||
|
|
@@ -133,10 +132,14 @@ | |
|
|
||
| bodyBuilder.Emit(OpCodes.Ldarg_0); | ||
| bodyBuilder.Emit(OpCodes.Call, | ||
| typeof(ClassInjectorBase).GetMethod(nameof(ClassInjectorBase.GetMonoObjectFromIl2CppPointer))!); | ||
| bodyBuilder.Emit(OpCodes.Castclass, typeof(Il2CppToMonoDelegateReference)); | ||
| typeof(Il2CppObjectPool).GetMethod(nameof(Il2CppObjectPool.Get))! | ||
| .MakeGenericMethod(typeof(Il2CppToMonoDelegateReference))); | ||
| bodyBuilder.Emit(OpCodes.Ldfld, | ||
| typeof(Il2CppToMonoDelegateReference).GetField(nameof(Il2CppToMonoDelegateReference.ReferencedDelegate))); | ||
| typeof(Il2CppToMonoDelegateReference).GetField(nameof(Il2CppToMonoDelegateReference.MethodInfo))); | ||
| bodyBuilder.Emit(OpCodes.Call, | ||
| typeof(Il2CppValueField<IntPtr>).GetMethod(nameof(Il2CppValueField<IntPtr>.Get))); | ||
| bodyBuilder.Emit(OpCodes.Call, | ||
| typeof(DelegateSupport).GetMethod(nameof(DelegateSupport.GetStoredDelegate), BindingFlags.NonPublic | BindingFlags.Static)); | ||
|
|
||
| for (var i = 0; i < managedParameters.Length; i++) | ||
| { | ||
|
|
@@ -190,15 +193,10 @@ | |
| bodyBuilder.Emit(OpCodes.Stloc, returnLocal); | ||
| } | ||
|
|
||
| var exceptionLocal = bodyBuilder.DeclareLocal(typeof(Exception)); | ||
| bodyBuilder.BeginCatchBlock(typeof(Exception)); | ||
| bodyBuilder.Emit(OpCodes.Stloc, exceptionLocal); | ||
| bodyBuilder.Emit(OpCodes.Ldstr, "Exception in IL2CPP-to-Managed trampoline, not passing it to il2cpp: "); | ||
| bodyBuilder.Emit(OpCodes.Ldloc, exceptionLocal); | ||
| bodyBuilder.Emit(OpCodes.Callvirt, typeof(object).GetMethod(nameof(ToString))!); | ||
| bodyBuilder.Emit(OpCodes.Call, | ||
| typeof(string).GetMethod(nameof(string.Concat), new[] { typeof(string), typeof(string) })!); | ||
| bodyBuilder.Emit(OpCodes.Call, typeof(DelegateSupport).GetMethod(nameof(LogError), BindingFlags.Static | BindingFlags.NonPublic)!); | ||
| bodyBuilder.Emit(OpCodes.Ldstr, "IL2CPP-to-Managed delegate trampoline"); | ||
| bodyBuilder.Emit(OpCodes.Call, typeof(ClassInjector).GetMethod(nameof(ClassInjector.LogException), | ||
| BindingFlags.Static | BindingFlags.NonPublic)!); | ||
|
|
||
| bodyBuilder.EndExceptionBlock(); | ||
|
|
||
|
|
@@ -209,11 +207,6 @@ | |
| return trampoline.CreateDelegate(GetOrCreateDelegateType(signature, managedMethod)); | ||
| } | ||
|
|
||
| private static void LogError(string message) | ||
| { | ||
| Logger.Instance.LogError("{Message}", message); | ||
| } | ||
|
|
||
| public static TIl2Cpp? ConvertDelegate<TIl2Cpp>(Delegate @delegate) where TIl2Cpp : Il2CppObjectBase | ||
| { | ||
| if (@delegate == null) | ||
|
|
@@ -289,6 +282,8 @@ | |
| methodInfo.IsMarshalledFromNative = true; | ||
|
|
||
| var delegateReference = new Il2CppToMonoDelegateReference(@delegate, methodInfo.Pointer); | ||
| // Leak the object so we never have to do this again. | ||
| GCHandle.Alloc(delegateReference); | ||
|
|
||
| Il2CppSystem.Delegate converted; | ||
| if (UnityVersionHandler.MustUseDelegateConstructor) | ||
|
|
@@ -317,6 +312,9 @@ | |
| return converted.Cast<TIl2Cpp>(); | ||
| } | ||
|
|
||
| private static readonly ConcurrentDictionary<IntPtr, Delegate> s_storedDelegates = new(); | ||
| private static Delegate GetStoredDelegate(IntPtr methodInfoPtr) => s_storedDelegates[methodInfoPtr]; | ||
|
|
||
| internal class MethodSignature : IEquatable<MethodSignature> | ||
| { | ||
| public readonly bool ConstructedFromNative; | ||
|
|
@@ -362,14 +360,14 @@ | |
| return _hashCode; | ||
| } | ||
|
|
||
| public bool Equals(MethodSignature other) | ||
|
Check warning on line 363 in Il2CppInterop.Runtime/DelegateSupport.cs
|
||
| { | ||
| if (ReferenceEquals(null, other)) return false; | ||
| if (ReferenceEquals(this, other)) return true; | ||
| return _hashCode.GetHashCode() == other.GetHashCode(); | ||
| } | ||
|
|
||
| public override bool Equals(object obj) | ||
| { | ||
| if (ReferenceEquals(null, obj)) return false; | ||
| if (ReferenceEquals(this, obj)) return true; | ||
|
|
@@ -390,8 +388,7 @@ | |
|
|
||
| private class Il2CppToMonoDelegateReference : Object | ||
| { | ||
| public IntPtr MethodInfo; | ||
| public Delegate ReferencedDelegate; | ||
|
Comment on lines
-393
to
-394
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm very happy to see this. One of the things in v2 that was bothering me is that I wasn't sure what to do with these fields. I'm introducing a breaking change that injected classes can't have managed state.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah i think breaking change is the only way to fix the semantics. i have a branch somewhere for bepinex which fixes it there. |
||
| public Il2CppValueField<IntPtr> MethodInfo; | ||
|
|
||
| public Il2CppToMonoDelegateReference(IntPtr obj0) : base(obj0) | ||
| { | ||
|
|
@@ -402,15 +399,15 @@ | |
| { | ||
| ClassInjector.DerivedConstructorBody(this); | ||
|
|
||
| ReferencedDelegate = referencedDelegate; | ||
| MethodInfo = methodInfo; | ||
| MethodInfo!.Set(methodInfo); | ||
| s_storedDelegates[methodInfo] = referencedDelegate; | ||
| } | ||
|
|
||
| ~Il2CppToMonoDelegateReference() | ||
| private void Il2CppFinalize() | ||
| { | ||
| Marshal.FreeHGlobal(MethodInfo); | ||
| MethodInfo = IntPtr.Zero; | ||
| ReferencedDelegate = null; | ||
| MethodInfo.Set(IntPtr.Zero); | ||
| s_storedDelegates.TryRemove(MethodInfo, out _); | ||
| } | ||
| } | ||
| } | ||
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.
v2 replaces
[HideFromIl2Cpp]with an opt-in system. The user indicator for this would be: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.
not exactly, i think the attribute does make sense for consistency but it would still need special-casing and the injected class'
Finalizemethod would not directly translate to the one defined here.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.
The Name property applies in any situation where an Il2Cpp name differs from the generated name in managed code. In the case of finalizers, the v2 generator appends underscores, which is approximately the same as what you're doing.