Skip to content

Commit

Permalink
[monodroid] Return of the Strings (dotnet#4487)
Browse files Browse the repository at this point in the history
Fixes: dotnet#4415
Context: ce2bc68

Commit ce2bc68 optimized type mappings between managed types and Java
types in large part by removing strings from the managed -> JNI
mapping: instead of using an assembly-qualified *string* as a key,
the assembly MVID & type metadata token were used as keys.

This setup works reliably in Release apps, in which the assemblies
don't change.  In a commercial Debug with Fast Deployment situation,
it falls down badly because every change to any source code that's
built into a mapped assembly may cause the assembly to change its
MVID, and renaming of any type -- removing or adding a type -- will
rearrange the type definition table in the resulting assembly, thus
changing the type token ids (which are basically offsets into the
type definition table in the PE executable).  This is what may cause
an app to crash on the runtime with an exception similar to:

	android.runtime.JavaProxyThrowable: System.NotSupportedException: Cannot create instance of type 'com.glmsoftware.OBDNowProto.SettingsFragmentCompat': no Java peer type found.
	  at Java.Interop.JniPeerMembers+JniInstanceMethods..ctor (System.Type declaringType) [0x0004b] in <e3e4dfa992a7411b85acfe193481be3e>:0
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.GetConstructorsForType (System.Type declaringType) [0x00031] in <e3e4dfa992a7411b85acfe193481be3e>:0
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.StartCreateInstance (System.String constructorSignature, System.Type declaringType, Java.Interop.JniArgumentValue* parameters) [0x00038] in <e3e4dfa992a7411b85acfe193481be3e>:0
	  at AndroidX.Preference.PreferenceFragmentCompat..ctor () [0x00034] in <005e3ae6340747e1aea6d08b095cf286>:0
	  at com.glmsoftware.OBDNowProto.SettingsFragmentCompat..ctor () [0x00026] in <a8dbee4be1674aa08cce57b50f21e347>:0
	  at com.glmsoftware.OBDNowProto.SettingsActivity.OnCreate (Android.OS.Bundle bundle) [0x00083] in <a8dbee4be1674aa08cce57b50f21e347>:0
	  at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState) [0x00011] in <c56099afccf04721853684f376a89527>:0
	    at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.3(intptr,intptr,intptr)
	    at crc64596a13587a898911.SettingsActivity.n_onCreate(Native Method)
	    at crc64596a13587a898911.SettingsActivity.onCreate(SettingsActivity.java:40)
	    at android.app.Activity.performCreate(Activity.java:7825)
	    at android.app.Activity.performCreate(Activity.java:7814)
	    at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1306)
	    at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3245)
	    at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3409)
	    at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
	    at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
	    at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
	    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
	    at android.os.Handler.dispatchMessage(Handler.java:107)
	    at android.os.Looper.loop(Looper.java:214)
	    at android.app.ActivityThread.main(ActivityThread.java:7356)
	    at java.lang.reflect.Method.invoke(Native Method)
	    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
	    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

A "workaround" would be fully rebuild the application, which negates
the point to incremental builds and the inner-dev-loop cycle.

The fix is to partially revert ce2bc68 in the sense that it restores
the use of string-based type names for Java-to-Managed and
Managed-to-Java type lookups for ***Debug*** builds only.  Unlike the
pre-ce2bc689 world, only *one* copy of the set of string names is
present within the data structures, at a tiny (sub millisecond) expense
at the run time to fix up pointers between the two tables.


~~ File Formats ~~

All data in all file formats remains little-endian.

In Debug configuration builds, each assembly will have a corresponding
`*.typemap` file which will be loaded at runtime.

The file format in pseudo-C++:

	struct DebugTypemapFileHeader {
	    byte                                magic [4];              // "XATS"
	    uint32_t                            format_version;         // 2
	    uint32_t                            entry_count;
	    uint32_t                            java_type_name_width;
	    uint32_t                            managed_type_name_width;
	    uint32_t                            assembly_name_size;
	    byte                                assembly_name [assembly_name_size];
	    DebugTypemapFileJavaToManagedEntry  java_to_managed [entry_count];
	    DebugTypemapFileManagedToJavaEntry  managed_to_java [entry_count];
	}

	struct DebugTypemapFileJavaToManagedEntry {
	    byte                jni_name [DebugTypemapFileHeader::java_type_name_width];
	    uint32_t            managed_index;  // Index into DebugTypemapFileHeader::managed_to_java
	};

	struct DebugTypemapFileManagedToJavaEntry {
	    byte                managed_name [DebugTypemapFileHeader::java_type_name_width];
	    uint32_t            jni_index;      // Index into DebugTypemapFileHeader::java_to_managed
	};

`DebugTypemapFileHeader::java_type_name_width` and
`DebugTypemapFileHeader::managed_type_name_width` are the maximum
length + 1 (terminating NUL) for JNI names and assembly-qualified
managed names.

`DebugTypemapFileJavaToManagedEntry::jni_name` and
`DebugTypemapFileManagedToJavaEntry::managed_name` are NUL-padded.
  • Loading branch information
grendello authored Apr 6, 2020
1 parent 15a800d commit 7117414
Show file tree
Hide file tree
Showing 24 changed files with 804 additions and 405 deletions.
6 changes: 6 additions & 0 deletions Documentation/release-notes/4487.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#### Application behavior on device and emulator

* [GitHub 4415](https://github.com/xamarin/xamarin-android/issues/4415):
Starting in Xamarin.Android 10.2.100.7, *System.NotSupportedException:
Cannot create instance of type ... no Java peer type found* caused certain
apps to crash during launch after incremental deployments.
19 changes: 12 additions & 7 deletions src/Mono.Android/Android.Runtime/JNIEnv.cs
Original file line number Diff line number Diff line change
Expand Up @@ -670,8 +670,8 @@ public static string GetClassNameFromInstance (IntPtr jobject)
}
}

[DllImport ("__Internal", CallingConvention = CallingConvention.Cdecl)]
static extern IntPtr monodroid_typemap_managed_to_java (byte[] mvid, int token);
[MethodImplAttribute(MethodImplOptions.InternalCall)]
static extern unsafe IntPtr monodroid_typemap_managed_to_java (Type type, byte* mvid);

internal static void LogTypemapTrace (StackTrace st)
{
Expand All @@ -685,19 +685,24 @@ internal static void LogTypemapTrace (StackTrace st)
}
}

internal static string TypemapManagedToJava (Type type)
internal static unsafe string TypemapManagedToJava (Type type)
{
if (mvid_bytes == null)
mvid_bytes = new byte[16];

Span<byte> mvid = new Span<byte>(mvid_bytes);
byte[] mvid_slow = null;
var mvid = new Span<byte>(mvid_bytes);
byte[] mvid_data = null;
if (!type.Module.ModuleVersionId.TryWriteBytes (mvid)) {
monodroid_log (LogLevel.Warn, LogCategories.Default, $"Failed to obtain module MVID using the fast method, falling back to the slow one");
mvid_slow = type.Module.ModuleVersionId.ToByteArray ();
mvid_data = type.Module.ModuleVersionId.ToByteArray ();
} else {
mvid_data = mvid_bytes;
}

IntPtr ret = monodroid_typemap_managed_to_java (mvid_slow == null ? mvid_bytes : mvid_slow, type.MetadataToken);
IntPtr ret;
fixed (byte* mvidptr = mvid_data) {
ret = monodroid_typemap_managed_to_java (type, mvidptr);
}

if (ret == IntPtr.Zero) {
if (LogTypemapMissStackTrace) {
Expand Down
11 changes: 4 additions & 7 deletions src/Mono.Android/Test/Java.Interop/JnienvTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,19 +383,16 @@ public void JavaToManagedTypeMapping ()
Assert.AreEqual (null, m);
}

[DllImport ("__Internal", CallingConvention = CallingConvention.Cdecl)]
static extern IntPtr monodroid_typemap_managed_to_java (byte[] mvid, int token);

[Test]
public void ManagedToJavaTypeMapping ()
{
Type type = typeof(Activity);
var m = monodroid_typemap_managed_to_java (type.Module.ModuleVersionId.ToByteArray (), type.MetadataToken);
Assert.AreNotEqual (IntPtr.Zero, m, "`Activity` subclasses Java.Lang.Object, it should be in the typemap!");
string m = JNIEnv.TypemapManagedToJava (type);
Assert.AreNotEqual (null, m, "`Activity` subclasses Java.Lang.Object, it should be in the typemap!");

type = typeof (JnienvTest);
m = monodroid_typemap_managed_to_java (type.Module.ModuleVersionId.ToByteArray (), type.MetadataToken);
Assert.AreEqual (IntPtr.Zero, m, "`JnienvTest` does *not* subclass Java.Lang.Object, it should *not* be in the typemap!");
m = JNIEnv.TypemapManagedToJava (type);
Assert.AreEqual (null, m, "`JnienvTest` does *not* subclass Java.Lang.Object, it should *not* be in the typemap!");
}

[Test]
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ void SaveResource (string resource, string filename, string destDir, Func<string
void WriteTypeMappings (List<TypeDefinition> types)
{
var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis);
if (!tmg.Generate (SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
throw new XamarinAndroidException (4308, Properties.Resources.XA4308);
GeneratedBinaryTypeMaps = tmg.GeneratedBinaryTypeMaps.ToArray ();
BuildEngine4.RegisterTaskObject (ApplicationConfigTaskState.RegisterTaskObjectKey, appConfState, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false);
Expand Down
14 changes: 11 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/PrepareAbiItems.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ public class PrepareAbiItems : AndroidTask
[Required]
public bool TypeMapMode { get; set; }

[Required]
public bool Debug { get; set; }

[Required]
public bool InstantRunEnabled { get; set; }

[Output]
public ITaskItem[] AssemblySources { get; set; }

Expand Down Expand Up @@ -53,9 +59,11 @@ public override bool RunTask ()
if (!TypeMapMode)
continue;

item = new TaskItem (Path.Combine (NativeSourcesDir, $"{baseName}.{abi}-managed.inc"));
item.SetMetadata ("abi", abi);
includes.Add (item);
if (!InstantRunEnabled && !Debug) {
item = new TaskItem (Path.Combine (NativeSourcesDir, $"{baseName}.{abi}-managed.inc"));
item.SetMetadata ("abi", abi);
includes.Add (item);
}
}

if (haveArmV7SharedSource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,6 @@ public void GenerateJavaStubsAndAssembly ([Values (true, false)] bool isRelease)
readonly string [] ExpectedAssemblyFiles = new [] {
Path.Combine ("android", "environment.armeabi-v7a.o"),
Path.Combine ("android", "environment.armeabi-v7a.s"),
Path.Combine ("android", "typemaps.armeabi-v7a-managed.inc"),
Path.Combine ("android", "typemaps.armeabi-v7a-shared.inc"),
Path.Combine ("android", "typemaps.armeabi-v7a.o"),
Path.Combine ("android", "typemaps.armeabi-v7a.s"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ARMNativeAssemblerTargetProvider : NativeAssemblerTargetProvider
public override string AbiName => Is64Bit ? ARMV8a : ARMV7a;
public override uint MapModulesAlignBits => Is64Bit ? 3u : 2u;
public override uint MapJavaAlignBits { get; } = 2;
public override uint DebugTypeMapAlignBits => Is64Bit ? 3u : 2u;

public ARMNativeAssemblerTargetProvider (bool is64Bit)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ abstract class NativeAssemblerTargetProvider
public abstract string AbiName { get; }
public abstract uint MapModulesAlignBits { get; }
public abstract uint MapJavaAlignBits { get; }
public abstract uint DebugTypeMapAlignBits { get; }

public virtual string MapType <T> ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ namespace Xamarin.Android.Tasks
{
class NativeTypeMappingData
{
public TypeMapGenerator.ModuleData[] Modules { get; }
public TypeMapGenerator.ModuleReleaseData[] Modules { get; }
public IDictionary<string, string> AssemblyNames { get; }
public string[] JavaTypeNames { get; }
public TypeMapGenerator.TypeMapEntry[] JavaTypes { get; }
public TypeMapGenerator.TypeMapReleaseEntry[] JavaTypes { get; }

public uint MapModuleCount { get; }
public uint JavaTypeCount { get; }
public uint JavaNameWidth { get; }

public NativeTypeMappingData (Action<string> logger, TypeMapGenerator.ModuleData[] modules, int javaNameWidth)
public NativeTypeMappingData (Action<string> logger, TypeMapGenerator.ModuleReleaseData[] modules, int javaNameWidth)
{
Modules = modules ?? throw new ArgumentNullException (nameof (modules));

Expand All @@ -24,19 +24,19 @@ public NativeTypeMappingData (Action<string> logger, TypeMapGenerator.ModuleData

AssemblyNames = new Dictionary<string, string> (StringComparer.Ordinal);

var tempJavaTypes = new Dictionary<string, TypeMapGenerator.TypeMapEntry> (StringComparer.Ordinal);
var tempJavaTypes = new Dictionary<string, TypeMapGenerator.TypeMapReleaseEntry> (StringComparer.Ordinal);
int managedStringCounter = 0;
var moduleComparer = new TypeMapGenerator.ModuleUUIDArrayComparer ();

foreach (TypeMapGenerator.ModuleData data in modules) {
foreach (TypeMapGenerator.ModuleReleaseData data in modules) {
data.AssemblyNameLabel = $"map_aname.{managedStringCounter++}";
AssemblyNames.Add (data.AssemblyNameLabel, data.AssemblyName);

int moduleIndex = Array.BinarySearch (modules, data, moduleComparer);
if (moduleIndex < 0)
throw new InvalidOperationException ($"Unable to map module with MVID {data.Mvid} to array index");

foreach (TypeMapGenerator.TypeMapEntry entry in data.Types) {
foreach (TypeMapGenerator.TypeMapReleaseEntry entry in data.Types) {
entry.ModuleIndex = moduleIndex;
if (tempJavaTypes.ContainsKey (entry.JavaName))
continue;
Expand All @@ -47,7 +47,7 @@ public NativeTypeMappingData (Action<string> logger, TypeMapGenerator.ModuleData
var javaNames = tempJavaTypes.Keys.ToArray ();
Array.Sort (javaNames, StringComparer.Ordinal);

var javaTypes = new TypeMapGenerator.TypeMapEntry[javaNames.Length];
var javaTypes = new TypeMapGenerator.TypeMapReleaseEntry[javaNames.Length];
for (int i = 0; i < javaNames.Length; i++) {
javaTypes[i] = tempJavaTypes[javaNames[i]];
}
Expand Down
Loading

0 comments on commit 7117414

Please sign in to comment.