From e88cfbcf9ff4f4e179a60f1e8af2bef3537a66b4 Mon Sep 17 00:00:00 2001 From: Brendan Zagaeski Date: Fri, 18 Dec 2020 12:08:50 -0500 Subject: [PATCH] [linker] Add AddKeepAlivesStep (#5278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/java.interop/issues/719 Context: https://github.com/xamarin/Java.Interop/commit/1f21f38c4f1b5d833565156da570853ffc58ae92 Context: https://docs.microsoft.com/archive/blogs/cbrumme/lifetime-gc-keepalive-handle-recycling We discovered the cause of a latent potential crash within Xamarin.Android apps: JNI DETECTED ERROR IN APPLICATION: use of deleted global reference 0x3d86 … The cause of the "use of deleted global reference" was that the GC collected an instance after it was provided to Java, but before Java could keep the instance alive in a manner which would cause our GC bridge to keep it alive, akin to: CallIntoJava (new JavaLangObjectSubclass ().Handle); In the above example code, if the `JavaLangObjectSubclass` instance is collected by the GC *after* the `.Handle` property is accessed but *before* `CallIntoJava()` is executed, then the `.Handle` value will refer to a "deleted global reference" and cause the app crash. The appropriate fix is to ensure that the GC *won't* collect it, usually by calling [`GC.KeepAlive()`][0]: var value = new JavaLangObjectSubclass (); CallIntoJava (value.Handle); GC.KeepAlive (value); An important aspect of the problem is that much of the relevant code introducing this scenario is within *binding assemblies*: partial class Activity { public virtual unsafe Android.App.PendingIntent? CreatePendingResult ( int requestCode, Android.Content.Intent data, [global::Android.Runtime.GeneratedEnum] Android.App.PendingIntentFlags flags) { const string __id = "createPendingResult.(ILandroid/content/Intent;I)Landroid/app/PendingIntent;"; try { JniArgumentValue* __args = stackalloc JniArgumentValue [3]; __args [0] = new JniArgumentValue (requestCode); __args [1] = new JniArgumentValue ((data == null) ? IntPtr.Zero : ((global::Java.Lang.Object) data).Handle); __args [2] = new JniArgumentValue ((int) flags); var __rm = _members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, __args); return global::Java.Lang.Object.GetObject (__rm.Handle, JniHandleOwnership.TransferLocalRef); } finally { } } } Here, the issue is that `data.Handle` is passed into Java code, but it's possible that `data` may be collected *after* `data.Handle` is accessed but before `_members.InstanceMethods.InvokeVirtualObjectMethod()` is invoked. xamarin/Java.Interop@1f21f38c introduced a `generator` fix for this this problem, inserting an appropriate `GC.KeepAlive()`: partial class Activity { public virtual unsafe Android.App.PendingIntent? CreatePendingResult ( int requestCode, Android.Content.Intent data, [global::Android.Runtime.GeneratedEnum] Android.App.PendingIntentFlags flags) { const string __id = "createPendingResult.(ILandroid/content/Intent;I)Landroid/app/PendingIntent;"; try { JniArgumentValue* __args = stackalloc JniArgumentValue [3]; __args [0] = new JniArgumentValue (requestCode); __args [1] = new JniArgumentValue ((data == null) ? IntPtr.Zero : ((global::Java.Lang.Object) data).Handle); __args [2] = new JniArgumentValue ((int) flags); var __rm = _members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, __args); return global::Java.Lang.Object.GetObject (__rm.Handle, JniHandleOwnership.TransferLocalRef); } finally { GC.KeepAlive (data); } } } The problem is that a fix within *generator* is meaningless until all relevant binding assemblies are *also* updated: fixing the issue within `Mono.Android.dll` only fixes `Mono.Android.dll`! We don't -- and can't! -- expect the entire ecosystem to rebuild and republish binding assemblies, which means a `generator` fix isn't a complete fix! To help fix the *ecosystem*, update the *linker* to insert appropriate `GC.KeepAlive()` invocations into marshal methods. This allows fixing the "problem domain" -- premature instance collection -- without requiring that the entire ecosystem be rebuilt. Instead, it "merely" requires that all *applications* be rebuilt. Add a new `$(AndroidAddKeepAlives)` MSBuild property which controls whether or not the linker inserts the `GC.KeepAlive()` calls. `$(AndroidAddKeepAlives)` defaults to True for Release configuration apps, and is False by default for "fast deployment" Debug builds. `$(AndroidAddKeepAlives)` can be overridden to explicitly enable or disable the new linker steps. The impact on release build of XA forms template (flyout variety) is cca 137ms on @radekdoulik's machine. * Disabled: 12079 ms LinkAssemblies 1 calls * Enabled: 12216 ms LinkAssemblies 1 calls Co-authored-by: Radek Doulik [0]: https://docs.microsoft.com/dotnet/api/system.gc.keepalive --- .../guides/building-apps/build-properties.md | 10 ++ Documentation/release-notes/5278.md | 5 + .../Microsoft.Android.Sdk.ILLink.csproj | 1 + .../MonoDroid.Tuner/AddKeepAlivesStep.cs | 130 ++++++++++++++++++ .../Linker/MonoDroid.Tuner/Extensions.cs | 70 ++++++++++ .../Linker/MonoDroid.Tuner/Linker.cs | 4 + .../Linker/MonoDroid.Tuner/LinkerOptions.cs | 1 + .../MonoDroid.Tuner/MonoDroidMarkStep.cs | 112 +++------------ .../Tasks/LinkAssemblies.cs | 3 + .../Tasks/LinkAssembliesNoShrink.cs | 13 +- .../Tasks/LinkerTests.cs | 68 +++++++++ .../Xamarin.Android.Common.targets | 3 + .../Xamarin.Android.Legacy.targets | 1 + 13 files changed, 327 insertions(+), 94 deletions(-) create mode 100644 Documentation/release-notes/5278.md create mode 100644 src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs diff --git a/Documentation/guides/building-apps/build-properties.md b/Documentation/guides/building-apps/build-properties.md index 6b3e75ff7ce..3f1aa0ec2b3 100644 --- a/Documentation/guides/building-apps/build-properties.md +++ b/Documentation/guides/building-apps/build-properties.md @@ -50,6 +50,16 @@ processing Android assets and resources. Added in Xamarin.Android 9.1. +## AndroidAddKeepAlives + +A boolean property that controls whether the linker will insert +`GC.KeepAlive()` invocations within binding projects to prevent premature +object collection. + +Defaults to `True` for Release configuration builds. + +Added in Xamarin.Android 11.2. + ## AndroidAotCustomProfilePath The file that `aprofutil` should create to hold profiler data. diff --git a/Documentation/release-notes/5278.md b/Documentation/release-notes/5278.md new file mode 100644 index 00000000000..99ad21c9b84 --- /dev/null +++ b/Documentation/release-notes/5278.md @@ -0,0 +1,5 @@ +#### Application and library build and deployment + + * [GitHub PR 5278](https://github.com/xamarin/xamarin-android/pull/5278): + Update the linker in Release builds to insert `GC.KeepAlive()` invocations within + JNI marshal methods to prevent premature instance collection. diff --git a/src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj b/src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj index 82e7b6d69a4..8c7851e5b67 100644 --- a/src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj +++ b/src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj @@ -28,6 +28,7 @@ + diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs new file mode 100644 index 00000000000..8c73c0bba18 --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs @@ -0,0 +1,130 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +using Mono.Cecil; + +using Java.Interop.Tools.Cecil; + +using Mono.Linker; +using Mono.Linker.Steps; + +using Mono.Cecil.Cil; + +namespace MonoDroid.Tuner +{ + public class AddKeepAlivesStep : BaseStep + { + readonly TypeDefinitionCache cache; + + public AddKeepAlivesStep (TypeDefinitionCache cache) + { + this.cache = cache; + } + + protected override void ProcessAssembly (AssemblyDefinition assembly) + { + var action = Annotations.HasAction (assembly) ? Annotations.GetAction (assembly) : AssemblyAction.Skip; + if (action == AssemblyAction.Delete) + return; + + if (AddKeepAlives (assembly)) { + if (action == AssemblyAction.Skip || action == AssemblyAction.Copy) + Annotations.SetAction (assembly, AssemblyAction.Save); + } + } + + internal bool AddKeepAlives (AssemblyDefinition assembly) + { + if (!assembly.MainModule.HasTypeReference ("Java.Lang.Object")) + return false; + + bool changed = false; + List types = assembly.MainModule.Types.ToList (); + foreach (TypeDefinition type in assembly.MainModule.Types) + AddNestedTypes (types, type); + + foreach (TypeDefinition type in types) + if (MightNeedFix (type)) + changed |= AddKeepAlives (type); + + return changed; + } + + // Adapted from `MarkJavaObjects` + static void AddNestedTypes (List types, TypeDefinition type) + { + if (!type.HasNestedTypes) + return; + + foreach (var t in type.NestedTypes) { + types.Add (t); + AddNestedTypes (types, t); + } + } + + bool MightNeedFix (TypeDefinition type) + { + return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", cache); + } + + MethodDefinition methodKeepAlive = null; + + bool AddKeepAlives (TypeDefinition type) + { + bool changed = false; + foreach (MethodDefinition method in type.Methods) { + if (!method.CustomAttributes.Any (a => a.AttributeType.FullName == "Android.Runtime.RegisterAttribute")) + continue; + + if (method.Parameters.Count == 0) + continue; + + var processor = method.Body.GetILProcessor (); + var module = method.DeclaringType.Module; + var instructions = method.Body.Instructions; + var end = instructions.Last (); + if (end.Previous.OpCode == OpCodes.Endfinally) + end = end.Previous; + + var found = false; + for (int off = Math.Max (0, instructions.Count - 6); off < instructions.Count; off++) { + var current = instructions [off]; + if (current.OpCode == OpCodes.Call && current.Operand.ToString ().Contains ("System.GC::KeepAlive")) { + found = true; + break; + } + } + + if (found) + continue; + + for (int i = 0; i < method.Parameters.Count; i++) { + if (method.Parameters [i].ParameterType.IsValueType || method.Parameters [i].ParameterType.FullName == "System.String") + continue; + + changed = true; + + if (methodKeepAlive == null) + methodKeepAlive = Context.GetMethod ("mscorlib", "System.GC", "KeepAlive", new string [] { "System.Object" }); + + processor.InsertBefore (end, GetLoadArgumentInstruction (method.IsStatic ? i : i + 1, method.Parameters [i])); + processor.InsertBefore (end, Instruction.Create (OpCodes.Call, module.ImportReference (methodKeepAlive))); + } + } + return changed; + } + + // Adapted from src/Mono.Android.Export/Mono.CodeGeneration/CodeArgumentReference.cs + static Instruction GetLoadArgumentInstruction (int argNum, ParameterDefinition parameter) + { + switch (argNum) { + case 0: return Instruction.Create (OpCodes.Ldarg_0); + case 1: return Instruction.Create (OpCodes.Ldarg_1); + case 2: return Instruction.Create (OpCodes.Ldarg_2); + case 3: return Instruction.Create (OpCodes.Ldarg_3); + default: return Instruction.Create (OpCodes.Ldarg, parameter); + } + } + } +} diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs index cc64366d3bf..009ae4f8321 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs @@ -46,6 +46,76 @@ public static object GetSettableValue (this CustomAttributeArgument arg) return td != null ? td.FullName + "," + td.Module.Assembly.FullName : arg.Value; } +#if !NETCOREAPP + public static AssemblyDefinition GetAssembly (this LinkContext context, string assemblyName) + { + AssemblyDefinition ad; + context.TryGetLinkedAssembly (assemblyName, out ad); + return ad; + } + + public static TypeDefinition GetType (this LinkContext context, string assemblyName, string typeName) + { + AssemblyDefinition ad = context.GetAssembly (assemblyName); + return ad == null ? null : GetType (ad, typeName); + } + + public static MethodDefinition GetMethod (this LinkContext context, string ns, string typeName, string name, string [] parameters) + { + var type = context.GetType (ns, typeName); + if (type == null) + return null; + + return GetMethod (type, name, parameters); + } +#endif + + public static MethodDefinition GetMethod (TypeDefinition td, string name) + { + MethodDefinition method = null; + foreach (var md in td.Methods) { + if (md.Name == name) { + method = md; + break; + } + } + + return method; + } + + public static MethodDefinition GetMethod (TypeDefinition type, string name, string [] parameters) + { + MethodDefinition method = null; + foreach (var md in type.Methods) { + if (md.Name != name) + continue; + + if (md.Parameters.Count != parameters.Length) + continue; + + var equal = true; + for (int i = 0; i < parameters.Length; i++) { + if (md.Parameters [i].ParameterType.FullName != parameters [i]) { + equal = false; + break; + } + } + + if (!equal) + continue; + + method = md; + break; + } + + return method; + } + + public static TypeDefinition GetType (AssemblyDefinition assembly, string typeName) + { + return assembly.MainModule.GetType (typeName); + } + public static bool Implements (this TypeReference self, string interfaceName) { if (interfaceName == null) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs index e0ae29b226b..dc1f50b64b1 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs @@ -65,6 +65,8 @@ static Pipeline CreatePipeline (LinkerOptions options) if (options.LinkNone) { pipeline.AppendStep (new FixAbstractMethodsStep (cache)); + if (options.AddKeepAlives) + pipeline.AppendStep (new AddKeepAlivesStep (cache)); pipeline.AppendStep (new OutputStepWithTimestamps ()); return pipeline; } @@ -118,6 +120,8 @@ static Pipeline CreatePipeline (LinkerOptions options) if (!string.IsNullOrWhiteSpace (options.ProguardConfiguration)) pipeline.AppendStep (new GenerateProguardConfiguration (options.ProguardConfiguration)); pipeline.AppendStep (new StripEmbeddedLibraries ()); + if (options.AddKeepAlives) + pipeline.AppendStep (new AddKeepAlivesStep (cache)); // end monodroid specific pipeline.AppendStep (new RegenerateGuidStep ()); pipeline.AppendStep (new OutputStepWithTimestamps ()); diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkerOptions.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkerOptions.cs index 0db87cf994e..8f079b19d06 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkerOptions.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkerOptions.cs @@ -22,6 +22,7 @@ class LinkerOptions public bool DumpDependencies { get; set; } public string HttpClientHandlerType { get; set; } public string TlsProvider { get; set; } + public bool AddKeepAlives { get; set; } public bool PreserveJniMarshalMethods { get; set; } public bool DeterministicOutput { get; set; } } diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs index 02df8a915e3..32b29ecdaa1 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs @@ -39,7 +39,7 @@ bool UpdateMarshalTypes () var updated = false; var typeName = "Java.Interop.TypeManager/JavaTypeManager/__<$>_jni_marshal_methods"; - var javaTypeManager = GetType ("Mono.Android", typeName); + var javaTypeManager = _context.GetType ("Mono.Android", typeName); if (javaTypeManager == null) throw new InvalidOperationException ($"Unable to find '{typeName}' in Mono.Android.dll!"); marshalTypes.Add (javaTypeManager); @@ -73,56 +73,6 @@ bool UpdateMarshalTypes () return updated; } - MethodDefinition GetMethod (TypeDefinition td, string name) - { - MethodDefinition method = null; - foreach (var md in td.Methods) { - if (md.Name == name) { - method = md; - break; - } - } - - return method; - } - - MethodDefinition GetMethod (string ns, string typeName, string name, string[] parameters) - { - var type = GetType (ns, typeName); - if (type == null) - return null; - - return GetMethod (type, name, parameters); - } - - MethodDefinition GetMethod (TypeDefinition type, string name, string[] parameters) - { - MethodDefinition method = null; - foreach (var md in type.Methods) { - if (md.Name != name) - continue; - - if (md.Parameters.Count != parameters.Length) - continue; - - var equal = true; - for (int i = 0; i < parameters.Length; i++) { - if (md.Parameters [i].ParameterType.FullName != parameters [i]) { - equal = false; - break; - } - } - - if (!equal) - continue; - - method = md; - break; - } - - return method; - } - MethodReference CreateGenericMethodReference (MethodReference method, GenericInstanceType type) { var genericMethod = new MethodReference (method.Name, method.ReturnType) { @@ -146,10 +96,10 @@ void UpdateRegistrationSwitch (MethodDefinition method, MethodReference[] switch instructions.Clear (); - var typeNullable = GetType ("mscorlib", "System.Nullable`1"); - var methodGetValueOrDefault = GetMethod (typeNullable, "GetValueOrDefault"); + var typeNullable = _context.GetType ("mscorlib", "System.Nullable`1"); + var methodGetValueOrDefault = Extensions.GetMethod (typeNullable, "GetValueOrDefault"); var genericTypeNullable = new GenericInstanceType (typeNullable); - genericTypeNullable.GenericArguments.Add (GetType ("mscorlib", "System.Int32")); + genericTypeNullable.GenericArguments.Add (_context.GetType ("mscorlib", "System.Int32")); var typeIdxVariable = new VariableDefinition (module.ImportReference (genericTypeNullable)); method.Body.Variables.Clear (); @@ -165,8 +115,8 @@ void UpdateRegistrationSwitch (MethodDefinition method, MethodReference[] switch for (var i = 0; i < switchMethods.Length; i++) switchInstructions [i] = Instruction.Create (OpCodes.Ldtoken, switchMethods [i].DeclaringType); - var typeType = GetType ("mscorlib", "System.Type"); - var methodGetTypeFromHandle = GetMethod ("mscorlib", "System.Type", "GetTypeFromHandle", new string[] { "System.RuntimeTypeHandle" }); + var typeType = _context.GetType ("mscorlib", "System.Type"); + var methodGetTypeFromHandle = _context.GetMethod ("mscorlib", "System.Type", "GetTypeFromHandle", new string[] { "System.RuntimeTypeHandle" }); var callDelegateStart = Instruction.Create (OpCodes.Call, module.ImportReference (methodGetTypeFromHandle)); instructions.Add (Instruction.Create (OpCodes.Switch, switchInstructions)); @@ -179,19 +129,19 @@ void UpdateRegistrationSwitch (MethodDefinition method, MethodReference[] switch instructions.Add (Instruction.Create (OpCodes.Ldc_I4_0)); instructions.Add (Instruction.Create (OpCodes.Ret)); - var actionType = GetType ("mscorlib", "System.Action`1"); + var actionType = _context.GetType ("mscorlib", "System.Action`1"); var genericActionType = new GenericInstanceType (actionType); - var argsType = GetType ("Java.Interop", "Java.Interop.JniNativeMethodRegistrationArguments"); + var argsType = _context.GetType ("Java.Interop", "Java.Interop.JniNativeMethodRegistrationArguments"); genericActionType.GenericArguments.Add (argsType); MarkType (genericActionType); - var actionInvoke = GetMethod (actionType, "Invoke", new string[] { "T" }); - var methodGetMethod = GetMethod ("mscorlib", "System.Type", "GetMethod", new string[] { "System.String" }); - var typeMethodInfo = GetType ("mscorlib", "System.Reflection.MethodInfo"); - var methodCreateDelegate = GetMethod ("mscorlib", "System.Reflection.MethodInfo", "CreateDelegate", new string[] { "System.Type" }); + var actionInvoke = Extensions.GetMethod (actionType, "Invoke", new string[] { "T" }); + var methodGetMethod = _context.GetMethod ("mscorlib", "System.Type", "GetMethod", new string[] { "System.String" }); + var typeMethodInfo = _context.GetType ("mscorlib", "System.Reflection.MethodInfo"); + var methodCreateDelegate = _context.GetMethod ("mscorlib", "System.Reflection.MethodInfo", "CreateDelegate", new string[] { "System.Type" }); instructions.Add (callDelegateStart); @@ -220,16 +170,16 @@ void UpdateMagicPrefill (TypeDefinition magicType) if (fieldTypesMap == null) return; - var methodPrefill = GetMethod (magicType, "Prefill"); + var methodPrefill = Extensions.GetMethod (magicType, "Prefill"); if (methodPrefill == null) return; - var typeDictionary = GetType ("mscorlib", "System.Collections.Generic.Dictionary`2"); - var ctorDictionary = GetMethod (typeDictionary, ".ctor", new string[] { "System.Int32" }); - var methodSetItem = GetMethod (typeDictionary, "set_Item", new string[] { "TKey", "TValue" }); + var typeDictionary = _context.GetType ("mscorlib", "System.Collections.Generic.Dictionary`2"); + var ctorDictionary = Extensions.GetMethod (typeDictionary, ".ctor", new string[] { "System.Int32" }); + var methodSetItem = Extensions.GetMethod (typeDictionary, "set_Item", new string[] { "TKey", "TValue" }); var genericTypeDictionary = new GenericInstanceType (typeDictionary); - genericTypeDictionary.GenericArguments.Add (GetType ("mscorlib", "System.String")); - genericTypeDictionary.GenericArguments.Add (GetType ("mscorlib", "System.Int32")); + genericTypeDictionary.GenericArguments.Add (_context.GetType ("mscorlib", "System.String")); + genericTypeDictionary.GenericArguments.Add (_context.GetType ("mscorlib", "System.Int32")); var genericMethodDictionaryCtor = CreateGenericMethodReference (ctorDictionary, genericTypeDictionary); var genericMethodDictionarySetItem = CreateGenericMethodReference (methodSetItem, genericTypeDictionary); @@ -256,11 +206,11 @@ void UpdateMagicPrefill (TypeDefinition magicType) void UpdateMagicRegistration () { - TypeDefinition magicType = GetType ("Mono.Android", "Android.Runtime.AndroidTypeManager/MagicRegistrationMap"); + TypeDefinition magicType = _context.GetType ("Mono.Android", "Android.Runtime.AndroidTypeManager/MagicRegistrationMap"); if (magicType == null) return; - MethodDefinition magicCall = GetMethod (magicType, "CallRegisterMethodByIndex"); + MethodDefinition magicCall = Extensions.GetMethod (magicType, "CallRegisterMethodByIndex"); if (magicCall == null) return; @@ -268,7 +218,7 @@ void UpdateMagicRegistration () var module = magicType.Module; int idx = 0; foreach (var type in marshalTypes) { - var md = GetMethod (type, "__RegisterNativeMembers"); + var md = Extensions.GetMethod (type, "__RegisterNativeMembers"); if (md == null) return; @@ -594,24 +544,6 @@ void ProcessSystemCore (TypeDefinition type) } } - protected AssemblyDefinition GetAssembly (string assemblyName) - { - AssemblyDefinition ad; - _context.TryGetLinkedAssembly (assemblyName, out ad); - return ad; - } - - protected TypeDefinition GetType (string assemblyName, string typeName) - { - AssemblyDefinition ad = GetAssembly (assemblyName); - return ad == null ? null : GetType (ad, typeName); - } - - protected TypeDefinition GetType (AssemblyDefinition assembly, string typeName) - { - return assembly.MainModule.GetType (typeName); - } - void ProcessSystemData (TypeDefinition type) { switch (type.Namespace) { @@ -619,7 +551,7 @@ void ProcessSystemData (TypeDefinition type) switch (type.Name) { case "SqlXml": // TODO: Needed only if CreateSqlReaderDelegate is used - TypeDefinition xml_reader = GetType ("System.Xml", "System.Xml.XmlReader"); + TypeDefinition xml_reader = _context.GetType ("System.Xml", "System.Xml.XmlReader"); MarkNamedMethod (xml_reader, "CreateSqlReader"); break; } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs index 16525473961..a401482ce84 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs @@ -49,6 +49,8 @@ public class LinkAssemblies : AndroidTask, ML.ILogger public string TlsProvider { get; set; } + public bool AddKeepAlives { get; set; } + public bool PreserveJniMarshalMethods { get; set; } public bool Deterministic { get; set; } @@ -100,6 +102,7 @@ bool Execute (DirectoryAssemblyResolver res) options.DumpDependencies = DumpDependencies; options.HttpClientHandlerType = HttpClientHandlerType; options.TlsProvider = TlsProvider; + options.AddKeepAlives = AddKeepAlives; options.PreserveJniMarshalMethods = PreserveJniMarshalMethods; options.DeterministicOutput = Deterministic; diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs index ed3651cd840..fb41cd4b557 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs @@ -28,6 +28,8 @@ public class LinkAssembliesNoShrink : AndroidTask [Required] public ITaskItem [] DestinationFiles { get; set; } + public bool AddKeepAlives { get; set; } + public bool Deterministic { get; set; } public override bool RunTask () @@ -50,8 +52,10 @@ public override bool RunTask () resolver.SearchDirectories.Add (path); } - // Run the FixAbstractMethodsStep - var step = new FixAbstractMethodsStep (resolver, new TypeDefinitionCache (), Log); + // Set up the FixAbstractMethodsStep + var step1 = new FixAbstractMethodsStep (resolver, new TypeDefinitionCache (), Log); + // Set up the AddKeepAlivesStep + var step2 = new MonoDroid.Tuner.AddKeepAlivesStep (new TypeDefinitionCache ()); for (int i = 0; i < SourceFiles.Length; i++) { var source = SourceFiles [i]; var destination = DestinationFiles [i]; @@ -60,7 +64,7 @@ public override bool RunTask () var assemblyName = Path.GetFileNameWithoutExtension (source.ItemSpec); if (!MTProfile.IsSdkAssembly (assemblyName) && !MTProfile.IsProductAssembly (assemblyName)) { assemblyDefinition = resolver.GetAssembly (source.ItemSpec); - step.CheckAppDomainUsage (assemblyDefinition, (string msg) => Log.LogMessageFromText (msg, MessageImportance.High)); + step1.CheckAppDomainUsage (assemblyDefinition, (string msg) => Log.LogMessageFromText (msg, MessageImportance.High)); } // Only run the step on "MonoAndroid" assemblies @@ -68,7 +72,8 @@ public override bool RunTask () if (assemblyDefinition == null) assemblyDefinition = resolver.GetAssembly (source.ItemSpec); - if (step.FixAbstractMethods (assemblyDefinition)) { + if (step1.FixAbstractMethods (assemblyDefinition) || + (AddKeepAlives && step2.AddKeepAlives (assemblyDefinition))) { Log.LogDebugMessage ($"Saving modified assembly: {destination.ItemSpec}"); writerParameters.WriteSymbols = assemblyDefinition.MainModule.HasSymbols; assemblyDefinition.Write (destination.ItemSpec, writerParameters); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs index 80489d73508..9361676b814 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs @@ -288,5 +288,73 @@ public AttributedButtonStub (Context context) : base (context) Assert.IsTrue (b.Build (proj), "Building a project with a null attribute value should have succeded."); } } + + [Test] + [Category ("DotNetIgnore")] + public void AndroidAddKeepAlives () + { + var proj = new XamarinAndroidApplicationProject { + IsRelease = true, + OtherBuildItems = { + new BuildItem ("Compile", "Method.cs") { TextContent = () => @" +using System; +using Java.Interop; + +namespace UnnamedProject { + public class MyClass : Java.Lang.Object + { + [Android.Runtime.Register(""MyMethod"")] + public unsafe bool MyMethod (Android.OS.IBinder windowToken, [global::Android.Runtime.GeneratedEnum] Android.Views.InputMethods.HideSoftInputFlags flags) + { + const string __id = ""hideSoftInputFromWindow.(Landroid/os/IBinder;I)Z""; + try { + JniArgumentValue* __args = stackalloc JniArgumentValue [1]; + __args [0] = new JniArgumentValue ((windowToken == null) ? IntPtr.Zero : ((global::Java.Lang.Object) windowToken).Handle); + __args [1] = new JniArgumentValue ((int) flags); + var __rm = JniPeerMembers.InstanceMethods.InvokeAbstractBooleanMethod (__id, this, __args); + return __rm; + } finally { + } + } + } +}" + }, + } + }; + + proj.SetProperty ("AllowUnsafeBlocks", "True"); + + using (var b = CreateApkBuilder ()) { + Assert.IsTrue (b.Build (proj), "Building a project should have succeded."); + + var assemblyPath = b.Output.GetIntermediaryPath (Path.Combine ("android", "assets", "UnnamedProject.dll")); + using (var assembly = AssemblyDefinition.ReadAssembly (assemblyPath)) { + Assert.IsTrue (assembly != null); + + var td = assembly.MainModule.GetType ("UnnamedProject.MyClass"); + Assert.IsTrue (td != null); + + var mr = td.GetMethods ().Where (m => m.Name == "MyMethod").FirstOrDefault (); + Assert.IsTrue (mr != null); + + var md = mr.Resolve (); + Assert.IsTrue (md != null); + + bool hasKeepAliveCall = false; + foreach (var i in md.Body.Instructions) { + if (i.OpCode.Code != Mono.Cecil.Cil.Code.Call) + continue; + + if (!i.Operand.ToString ().Contains ("System.GC::KeepAlive")) + continue; + + hasKeepAliveCall = true; + break; + } + + Assert.IsTrue (hasKeepAliveCall); + } + } + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 31d3bdcca1d..41fd8284eef 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -842,6 +842,7 @@ because xbuild doesn't support framework reference assemblies. + True <_AndroidBuildPropertiesCacheExists Condition=" Exists('$(_AndroidBuildPropertiesCache)') ">True <_NuGetAssetsFile Condition=" Exists('$(ProjectLockFile)') ">$(ProjectLockFile) <_NuGetAssetsFile Condition=" '$(_NuGetAssetsFile)' == '' and Exists('packages.config') ">packages.config @@ -854,6 +855,7 @@ because xbuild doesn't support framework reference assemblies. <_PropertyCacheItems Include="BundleAssemblies=$(BundleAssemblies)" /> <_PropertyCacheItems Include="AotAssemblies=$(AotAssemblies)" /> + <_PropertyCacheItems Include="AndroidAddKeepAlives=$(AndroidAddKeepAlives)" /> <_PropertyCacheItems Include="AndroidAotMode=$(AndroidAotMode)" /> <_PropertyCacheItems Include="AndroidEmbedProfilers=$(AndroidEmbedProfilers)" /> <_PropertyCacheItems Include="AndroidEnableProfiledAot=$(AndroidEnableProfiledAot)" /> @@ -1327,6 +1329,7 @@ because xbuild doesn't support framework reference assemblies. ResolvedAssemblies="@(_AllResolvedAssemblies)" SourceFiles="@(ResolvedAssemblies)" DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" + AddKeepAlives="$(AndroidAddKeepAlives)" Deterministic="$(Deterministic)" /> diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets index 01333897e8b..7d4e8a5239e 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets @@ -637,6 +637,7 @@ projects. .NET 5 projects will not import this file. LinkSkip="$(AndroidLinkSkip)" LinkDescriptions="@(LinkDescription)" ProguardConfiguration="$(_ProguardProjectConfiguration)" + AddKeepAlives="$(AndroidAddKeepAlives)" PreserveJniMarshalMethods="$(AndroidGenerateJniMarshalMethods)" EnableProguard="$(AndroidEnableProguard)" Deterministic="$(Deterministic)"