[typemaps] Handle Java to managed type maps properly

src/Mono.Android/Test/Android.OS/BundleTest.cs

@@ -19,6 +19,7 @@ public void TestBundleIntegerArrayList()
 			Assert.NotNull (list, "'key' doesn't refer to a list of integers");
 			var obj = b.Get ("key");
 			Assert.NotNull (obj, "Missing 'key' in bundle");
+			Assert.IsTrue (obj is global::Android.Runtime.JavaList, "`obj` should be a JavaList!");
 			try {
 				list = b.GetIntegerArrayList ("key");
 				Assert.NotNull (list, "'key' doesn't refer to a list of integers after non-generic call");

src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs

@@ -174,7 +174,7 @@ void Run (DirectoryAssemblyResolver res)
 
 			// Step 2 - Generate type maps
 			//   Type mappings need to use all the assemblies, always.
-			WriteTypeMappings (allJavaTypes);
+			WriteTypeMappings (allJavaTypes, cache);
 
 			var javaTypes = new List<TypeDefinition> ();
 			foreach (TypeDefinition td in allJavaTypes) {
@@ -399,10 +399,10 @@ void SaveResource (string resource, string filename, string destDir, Func<string
 			MonoAndroidHelper.CopyIfStringChanged (template, Path.Combine (destDir, filename));
 		}
 
-		void WriteTypeMappings (List<TypeDefinition> types)
+		void WriteTypeMappings (List<TypeDefinition> types, TypeDefinitionCache cache)
 		{
 			var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis);
-			if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
+			if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, cache, 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);

src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Text;
 
+using Java.Interop.Tools.Cecil;
 using Mono.Cecil;
 
 namespace Xamarin.Android.Tasks
@@ -14,6 +15,7 @@ class TypeMapGenerator
 		const string TypeMapIndexMagicString = "XATI"; // Xamarin Android Typemap Index
 		const uint TypeMapFormatVersion = 2; // Keep in sync with the value in src/monodroid/jni/xamarin-app.hh
 		const string TypemapExtension = ".typemap";
+		const uint InvalidJavaToManagedMappingIndex = UInt32.MaxValue;
 
 		internal sealed class ModuleUUIDArrayComparer : IComparer<ModuleReleaseData>
 		{
@@ -52,6 +54,7 @@ internal sealed class TypeMapReleaseEntry
 			public uint Token;
 			public int AssemblyNameIndex = -1;
 			public int ModuleIndex = -1;
+			public bool SkipInJavaToManaged;
 		}
 
 		internal sealed class ModuleReleaseData
@@ -76,6 +79,8 @@ internal sealed class TypeMapDebugEntry
 			public string ManagedLabel;
 			public int JavaIndex;
 			public int ManagedIndex;
+			public TypeDefinition TypeDefinition;
+			public bool SkipInJavaToManaged;
 		}
 
 		// Widths include the terminating nul character but not the padding!
@@ -87,6 +92,7 @@ internal sealed class ModuleDebugData
 			public List<TypeMapDebugEntry> JavaToManagedMap;
 			public List<TypeMapDebugEntry> ManagedToJavaMap;
 			public string OutputFilePath;
+			public string ModuleName;
 			public byte[] ModuleNameBytes;
 		}
 
@@ -125,7 +131,7 @@ void UpdateApplicationConfig (TypeDefinition javaType, ApplicationConfigTaskStat
 			}
 		}
 
-		public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState)
+		public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState)
 		{
 			if (String.IsNullOrEmpty (outputDirectory))
 				throw new ArgumentException ("must not be null or empty", nameof (outputDirectory));
@@ -140,25 +146,26 @@ public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAt
 			string typemapsOutputDirectory = Path.Combine (outputDirectory, "typemaps");
 
 			if (debugBuild) {
-				return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, generateNativeAssembly, appConfState);
+				return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, typemapsOutputDirectory, generateNativeAssembly, appConfState);
 			}
 
 			return GenerateRelease (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, appConfState);
 		}
 
-		bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState)
+		bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState)
 		{
 			if (generateNativeAssembly)
-				return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState);
-			return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState);
+				return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState);
+			return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState);
 		}
 
-		bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
+		bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState)
 		{
 			var modules = new Dictionary<string, ModuleDebugData> (StringComparer.Ordinal);
 			int maxModuleFileNameWidth = 0;
 			int maxModuleNameWidth = 0;
 
+			var javaDuplicates = new Dictionary<string, List<TypeMapDebugEntry>> (StringComparer.Ordinal);
 			foreach (TypeDefinition td in javaTypes) {
 				UpdateApplicationConfig (td, appConfState);
 				string moduleName = td.Module.Assembly.Name.Name;
@@ -173,7 +180,8 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
 						JavaToManagedMap = new List<TypeMapDebugEntry> (),
 						ManagedToJavaMap = new List<TypeMapDebugEntry> (),
 						OutputFilePath = Path.Combine (outputDirectory, outputFileName),
-						ModuleNameBytes = outputEncoding.GetBytes (moduleName)
+						ModuleName = moduleName,
+						ModuleNameBytes = outputEncoding.GetBytes (moduleName),
 					};
 
 					if (module.ModuleNameBytes.Length > maxModuleNameWidth)
@@ -186,6 +194,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
 				}
 
 				TypeMapDebugEntry entry = GetDebugEntry (td);
+				HandleDebugDuplicates (javaDuplicates, entry, td, cache);
 				if (entry.JavaName.Length > module.JavaNameWidth)
 					module.JavaNameWidth = (uint)entry.JavaName.Length + 1;
 
@@ -195,6 +204,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
 				module.JavaToManagedMap.Add (entry);
 				module.ManagedToJavaMap.Add (entry);
 			}
+			SyncDebugDuplicates (javaDuplicates);
 
 			foreach (ModuleDebugData module in modules.Values) {
 				PrepareDebugMaps (module);
@@ -217,18 +227,22 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
 			return true;
 		}
 
-		bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
+		bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState)
 		{
 			var javaToManaged = new List<TypeMapDebugEntry> ();
 			var managedToJava = new List<TypeMapDebugEntry> ();
 
+			var javaDuplicates = new Dictionary<string, List<TypeMapDebugEntry>> (StringComparer.Ordinal);
 			foreach (TypeDefinition td in javaTypes) {
 				UpdateApplicationConfig (td, appConfState);
 
 				TypeMapDebugEntry entry = GetDebugEntry (td);
+				HandleDebugDuplicates (javaDuplicates, entry, td, cache);
+
 				javaToManaged.Add (entry);
 				managedToJava.Add (entry);
 			}
+			SyncDebugDuplicates (javaDuplicates);
 
 			var data = new ModuleDebugData {
 				EntryCount = (uint)javaToManaged.Count,
@@ -246,6 +260,39 @@ bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttribu
 			return true;
 		}
 
+		void SyncDebugDuplicates (Dictionary<string, List<TypeMapDebugEntry>> javaDuplicates)
+		{
+			foreach (List<TypeMapDebugEntry> duplicates in javaDuplicates.Values) {
+				if (duplicates.Count < 2) {
+					continue;
+				}
+
+				TypeMapDebugEntry template = duplicates [0];
+				for (int i = 1; i < duplicates.Count; i++) {
+					duplicates[i].TypeDefinition = template.TypeDefinition;
+					duplicates[i].ManagedName = template.ManagedName;
+				}
+			}
+		}
+
+		void HandleDebugDuplicates (Dictionary<string, List<TypeMapDebugEntry>> javaDuplicates, TypeMapDebugEntry entry, TypeDefinition td, TypeDefinitionCache cache)
+		{
+			List<TypeMapDebugEntry> duplicates;
+
+			if (!javaDuplicates.TryGetValue (entry.JavaName, out duplicates)) {
+				javaDuplicates.Add (entry.JavaName, new List<TypeMapDebugEntry> { entry });
+			} else {
+				duplicates.Add (entry);
+				TypeMapDebugEntry oldEntry = duplicates[0];
+				if (td.IsAbstract || td.IsInterface || oldEntry.TypeDefinition.IsAbstract || oldEntry.TypeDefinition.IsInterface) {
+					if (td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
+						oldEntry.TypeDefinition = td;
+						oldEntry.ManagedName = GetManagedTypeName (td);
+					}
+				}
+			}
+		}
+
 		void PrepareDebugMaps (ModuleDebugData module)
 		{
 			module.JavaToManagedMap.Sort ((TypeMapDebugEntry a, TypeMapDebugEntry b) => String.Compare (a.JavaName, b.JavaName, StringComparison.Ordinal));
@@ -261,6 +308,16 @@ void PrepareDebugMaps (ModuleDebugData module)
 		}
 
 		TypeMapDebugEntry GetDebugEntry (TypeDefinition td)
+		{
+			return new TypeMapDebugEntry {
+				JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td),
+				ManagedName = GetManagedTypeName (td),
+				TypeDefinition = td,
+				SkipInJavaToManaged = ShouldSkipInJavaToManaged (td),
+			};
+		}
+
+		string GetManagedTypeName (TypeDefinition td)
 		{
 			// This is necessary because Mono runtime will return to us type name with a `.` for nested types (not a
 			// `/` or a `+`. So, for instance, a type named `DefaultRenderer` found in the
@@ -277,17 +334,13 @@ TypeMapDebugEntry GetDebugEntry (TypeDefinition td)
 			//
 			string managedTypeName = td.FullName.Replace ('/', '+');
 
-			return new TypeMapDebugEntry {
-				JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td),
-				ManagedName = $"{managedTypeName}, {td.Module.Assembly.Name.Name}",
-			};
+			return $"{managedTypeName}, {td.Module.Assembly.Name.Name}";
 		}
 
 		bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
 		{
 			int assemblyId = 0;
 			int maxJavaNameLength = 0;
-			int maxModuleFileNameLength = 0;
 			var knownAssemblies = new Dictionary<string, int> (StringComparer.Ordinal);
 			var tempModules = new Dictionary<byte[], ModuleReleaseData> ();
 			Dictionary <AssemblyDefinition, int> moduleCounter = null;
@@ -329,12 +382,18 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List
 				}
 
 				string javaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td);
+				// We will ignore generic types and interfaces when generating the Java to Managed map, but we must not
+				// omit them from the table we output - we need the same number of entries in both java-to-managed and
+				// managed-to-java tables.  `SkipInJavaToManaged` set to `true` will cause the native assembly generator
+				// to output `0` as the token id for the type, thus effectively causing the runtime unable to match such
+				// a Java type name to a managed type. This fixes https://github.com/xamarin/xamarin-android/issues/4660
 				var entry = new TypeMapReleaseEntry {
 					JavaName = javaName,
 					JavaNameLength = outputEncoding.GetByteCount (javaName),
 					ManagedTypeName = td.FullName,
 					Token = td.MetadataToken.ToUInt32 (),
-					AssemblyNameIndex = knownAssemblies [assemblyName]
+					AssemblyNameIndex = knownAssemblies [assemblyName],
+					SkipInJavaToManaged = ShouldSkipInJavaToManaged (td),
 				};
 
 				if (entry.JavaNameLength > maxJavaNameLength)
@@ -376,6 +435,11 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List
 			return true;
 		}
 
+		bool ShouldSkipInJavaToManaged (TypeDefinition td)
+		{
+			return td.IsInterface || td.HasGenericParameters;
+		}
+
 		void GenerateNativeAssembly (Func<NativeAssemblerTargetProvider, bool, bool, NativeAssemblyGenerator> getGenerator)
 		{
 			NativeAssemblerTargetProvider asmTargetProvider;
@@ -491,6 +555,10 @@ void OutputModule (ModuleDebugData moduleData)
 		//
 		void OutputModule (BinaryWriter bw, ModuleDebugData moduleData)
 		{
+			if ((uint)moduleData.JavaToManagedMap.Count == InvalidJavaToManagedMappingIndex) {
+				throw new InvalidOperationException ($"Too many types in module {moduleData.ModuleName}");
+			}
+
 			bw.Write (moduleMagicString);
 			bw.Write (TypeMapFormatVersion);
 			bw.Write (moduleData.JavaToManagedMap.Count);
@@ -502,7 +570,7 @@ void OutputModule (BinaryWriter bw, ModuleDebugData moduleData)
 			foreach (TypeMapDebugEntry entry in moduleData.JavaToManagedMap) {
 				bw.Write (outputEncoding.GetBytes (entry.JavaName));
 				PadField (bw, entry.JavaName.Length, (int)moduleData.JavaNameWidth);
-				bw.Write (entry.ManagedIndex);
+				bw.Write (entry.SkipInJavaToManaged ? InvalidJavaToManagedMappingIndex : (uint)entry.ManagedIndex);
 			}
 
 			foreach (TypeMapDebugEntry entry in moduleData.ManagedToJavaMap) {

src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs

@@ -70,7 +70,7 @@ protected override void WriteSymbols (StreamWriter output)
 			if (haveJavaToManaged) {
 				foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) {
 					size += WritePointer (output, entry.JavaLabel);
-					size += WritePointer (output, entry.ManagedLabel);
+					size += WritePointer (output, entry.SkipInJavaToManaged ? null : entry.ManagedLabel);
 				}
 			}
 			WriteStructureSize (output, JavaToManagedSymbol, size, alwaysWriteSize: true);

src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs

@@ -219,7 +219,7 @@ uint WriteJavaMapEntry (StreamWriter output, TypeMapGenerator.TypeMapReleaseEntr
 			size += WriteData (output, entry.ModuleIndex);
 
 			WriteCommentLine (output, "type_token_id");
-			size += WriteData (output, entry.Token);
+			size += WriteData (output, entry.SkipInJavaToManaged ? 0 : entry.Token);
 
 			WriteCommentLine (output, "java_name");
 			size += WriteAsciiData (output, entry.JavaName, mappingData.JavaNameWidth);

src/monodroid/jni/embedded-assemblies.cc

@@ -11,6 +11,7 @@
 #include <libgen.h>
 #include <errno.h>
 #include <unistd.h>
+#include <climits>
 
 #include <mono/metadata/assembly.h>
 #include <mono/metadata/image.h>
@@ -208,6 +209,10 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
 	}
 
 	const char *managed_type_name = entry->to;
+	if (managed_type_name == nullptr) {
+		log_debug (LOG_ASSEMBLY, "typemap: Java type '%s' maps either to an open generic type or an interface type.");
+		return nullptr;
+	}
 	log_debug (LOG_DEFAULT, "typemap: Java type '%s' corresponds to managed type '%s'", java_type_name, managed_type_name);
 
 	MonoType *type = mono_reflection_type_from_name (const_cast<char*>(managed_type_name), nullptr);
@@ -731,12 +736,18 @@ EmbeddedAssemblies::typemap_load_file (BinaryTypeMapHeader &header, const char *
 	uint8_t *managed_pos = managed_start;
 	TypeMapEntry *cur;
 
+	constexpr uint32_t INVALID_TYPE_INDEX = UINT32_MAX;
 	for (size_t i = 0; i < module.entry_count; i++) {
 		cur = &module.java_to_managed[i];
 		cur->from = reinterpret_cast<char*>(java_pos);
 
 		uint32_t idx = *(reinterpret_cast<uint32_t*>(java_pos + header.java_name_width));
-		cur->to = reinterpret_cast<char*>(managed_start + (managed_entry_size * idx));
+		if (idx < INVALID_TYPE_INDEX) {
+			cur->to = reinterpret_cast<char*>(managed_start + (managed_entry_size * idx));
+		} else {
+			// Ignore the type mapping
+			cur->to = nullptr;
+		}
 		java_pos += java_entry_size;
 
 		cur = &module.managed_to_java[i];

tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs

@@ -12,6 +12,13 @@ namespace Xamarin.Android.JcwGenTests {
 	[TestFixture]
 	public class BindingTests {
 
+		[Test]
+		public void TestTimingCreateTimingIsCorrectType ()
+		{
+			var t = Com.Xamarin.Android.Timing.CreateTiming ();
+			Assert.IsTrue (t is Com.Xamarin.Android.Timing);
+		}
+
 		[Test]
 		public void TestResourceId ()
 		{

tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs

@@ -8,6 +8,12 @@
 
 namespace Com.Xamarin.Android {
 
+	// Provoke https://github.com/xamarin/xamarin-android/issues/4660
+	// We need this type to appear *before* `Timing` in `monodis --typedef` output
+	[Register ("com/xamarin/android/Timing", DoNotGenerateAcw = true)]
+	public partial class Timinf<T> : Timing {
+	}
+
 	public partial class Timing {
 
 		static IntPtr jonp_id_VirtualVoidMethod;

tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/java/com/xamarin/android/Timing.java

@@ -4,6 +4,10 @@
 
 public class Timing {
 
+	public static Timing createTiming () {
+		return new Timing ();
+	}
+
 	public static void StaticVoidMethod ()
 	{
 	}