Skip to content
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

Can not Export CREATOR Field for IParcelable instance #1149

Closed
fullmetaltac opened this issue Jan 2, 2018 · 11 comments
Closed

Can not Export CREATOR Field for IParcelable instance #1149

fullmetaltac opened this issue Jan 2, 2018 · 11 comments
Assignees
Labels
Area: App Runtime Issues in `libmonodroid.so`. regression
Milestone

Comments

@fullmetaltac
Copy link

fullmetaltac commented Jan 2, 2018

Sample link

https://github.com/xamarin/monodroid-samples/blob/f944f42001fa134915069fa7bbb9cb493bfd446d/ExportAttribute/ExportAttributeTest/MainActivity.cs#L55
https://github.com/xamarin/monodroid-samples/blob/f944f42001fa134915069fa7bbb9cb493bfd446d/android5.0/Topeka/Topeka/Models/Player.cs#L18

Verified on Monodroid v8.2.0.1

Issue description

Can not ExportField CREATOR for IParcelable instance

Steps to reproduce the issue

  1. Open Topeka sample.
  2. Input login info.
  3. Tap Login button.

What's the expected result?

  • User successfully login.

What's the actual result?

  • Application crashes.

Steps to reproduce the issue

  1. Open ExportAttributeTest sample.
  2. Tap button.

What's the expected result?

  • Parcelable object handled properly.

What's the actual result?

  • Application crashes.

Additional details / screenshot (Optional)

image

InitializeCreator method is not reachable via debugger.
MSBuildOutput.log
logCat.txt

@jonpryor jonpryor added the Area: App Runtime Issues in `libmonodroid.so`. label Jan 2, 2018
@jonpryor
Copy link
Member

jonpryor commented Jan 2, 2018

This does not crash on d15-5/Xamarin.Android 8.1.

@fullmetaltac: Does this happen in the Debug configuration or when linking is disabled?

@radekdoulik: I suspect that this is a linker bug, and that Player.InitializeCreator() is being linked away, which would explain why it can't be found.

Could this be related to PR #1059?

@jonpryor jonpryor added this to the d15-6 milestone Jan 2, 2018
@fullmetaltac
Copy link
Author

@jonpryor I tried with Visual Studio Tools for Xamarin v4.9.0.669 and v4.8.0.753 both with Release and Debug configurations. I tested on real device Xiaomi Redmi 3S (v6.0.1) and with Nexus One (API:27) emulator. Result the same in each case.

Here is crash info from device. The same error I was getting spontaneously and was not able to write concrete steps to reproduce. This error getting periodically and I hope has the same root cause.
4596cb75-f6cf-478a-9bac-330bd8a9d345

@fullmetaltac
Copy link
Author

fullmetaltac commented Jan 3, 2018

@jonpryor Should I close this duplicate?
dotnet/android-samples#254

@radekdoulik
Copy link
Member

Looks like it is not Linker related. Happens in Debug configuration as well.

I will try to find out why it fails.

@radekdoulik
Copy link
Member

@jonathanpeppers Looks like it might be related to dotnet/java-interop@429dc2a

Do you know what might be wrong here? Looks like we might be looking for wrong signature?

@jonpryor
Copy link
Member

jonpryor commented Jan 8, 2018

@radekdoulik: @jonathanpeppers is OOF until ~march, so don't expect much from him. :-)

I also doubt that it's dotnet/java-interop@429dc2a, as the bug is reported against d15-6, and the referenced Java.Interop commit should only be in d15-7/master, not d15-6. (At least it better not be in d15-6; I wanted it to get more testing!)

I don't see that commit in https://github.com/xamarin/Java.Interop/commits/d15-6.

@radekdoulik: Do you have any additional information to bring to the table?

@radekdoulik
Copy link
Member

@jonpryor Indeed. Another candidate might be dotnet/java-interop@cb161d2

Trying to find out more about it.

@radekdoulik
Copy link
Member

The above mentioned commit indeed caused the regression.

The signature for the InitializeCreator signature is now ()Ljava/lang/Object; while it was ()Lmd59b7a7576784821ea63294fbca8600da1/Creator_1; before.

@jonpryor
Copy link
Member

jonpryor commented Jan 8, 2018

@radekdoulik: I think I have a theory. Consider JavaNativeTypeManager.ToJniNameFromAttributes(TypeDefinition). What happens if the TypeDefinition does not implement IJniNameProviderAttribute?

In particular, Creator<T> does not implement IJniNameProviderAttribute -- nor should it.

If the TypeDefinition (typeof(Creator<>)) does not implement IJniNameProviderAttribute, then JavaNativeTypeManager.ToJniNameFromAttributes(TypeDefinition) will return null, which means JavaNativeTypeManager.ToJniName(TypeDefinition) will return java/lang/Object, which explains the new signature you're seeing for InitializeCreator().

...which doesn't actually make sense; shouldn't the IJavaObject check in JavaNativeTypeManager.ToJniName(TypeDefinition, ExportKind) catch Creator<T>? In which case we should be hitting ToJniName<T>(), which should create the md5.../Creator_1 name.

More digging is needed. :-(

radekdoulik added a commit to radekdoulik/java.interop that referenced this issue Jan 8, 2018
Commit
dotnet@cb161d2
introduced a regression described in
dotnet/android#1149 (comment)

The signature of the `InitializeCreator` method was wrongly calculated
as `()Ljava/lang/Object;` instead of original
`()Lmd59b7a7576784821ea63294fbca8600da1/Creator_1;`

The reason for that is, that
dotnet@cb161d2#diff-9e153905b02dbe50c0e0b874ba40bf3eR262
started using `IEnumerable<T>::OfType<T>`, which traverses
inheritance, unlike the original `Type::GetCustomAttributes` called
with `inherit = false`.

Thus for the `Creator<Player>` we got the `RegisterAttribute` from the
parent type `Java.Lang.Object` which led to non-null result from
`ToJniNameFromAttributes` method. At the end the result was used here
dotnet@cb161d2#diff-9e153905b02dbe50c0e0b874ba40bf3eR361
instead of returning `ToJniName (type, exportKind)` result.
radekdoulik added a commit to radekdoulik/java.interop that referenced this issue Jan 8, 2018
Commit
dotnet@cb161d2
introduced a regression described in
dotnet/android#1149 (comment)

The signature of the `InitializeCreator` method was wrongly calculated
as `()Ljava/lang/Object;` instead of original
`()Lmd59b7a7576784821ea63294fbca8600da1/Creator_1;`

The reason for that is, that
dotnet@cb161d2#diff-9e153905b02dbe50c0e0b874ba40bf3eR262
started using `IEnumerable<T>::OfType<T>`, which traverses
inheritance, unlike the original `Type::GetCustomAttributes` called
with `inherit = false`.

Thus for the `Creator<Player>` we got the `RegisterAttribute` from the
parent type `Java.Lang.Object` which led to non-null result from
`ToJniNameFromAttributes` method. At the end the result was used here
dotnet@cb161d2#diff-9e153905b02dbe50c0e0b874ba40bf3eR361
instead of returning `ToJniName (type, exportKind)` result.
jonpryor pushed a commit to dotnet/java-interop that referenced this issue Jan 9, 2018
Fixes: dotnet/android#1149

Commit cb161d2 broke the use of `[Java.Interop.ExportAttribute]` and
`[Java.Interop.ExportFieldAttribute]`.

Repro:

 1. [Create a class with an `[Export]` or `[ExportField]` method][0]:

		partial class MyParcelable : Java.Lang.Object, IParcelable {
			[ExportField ("CREATOR")]
			public static MyParcelableCreator InitializeCreator ()
			{
				/* ... */
			}
		}

 2. Launch the app, and cause the `MyParcelable` Java Callable Wrapper (JCW)
    to be initialized:
    
		// Will eventually cause JCW MyParcelable.class to be init'd
		var whatever = new MyParcelable();

Expected result: It works!

Actual result:

	System.InvalidOperationException: Specified managed method 'InitializeCreator' was not found. Signature: ()Lmd5<<whatever>>/MyParcelableCreator;
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len) [0x001d2] in <36a2bb1be1a64b51b22cbef15eee5437>:0 
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_alloc_object(intptr,intptr&,intptr)
	  at Java.Interop.JniEnvironment+Object.AllocObject (Java.Interop.JniObjectReference type) [0x00027] in <8e80eb3ca41f4c3eb745bd7c73458796>:0 
	  at Java.Interop.JniType.AllocObject () [0x0000c] in <8e80eb3ca41f4c3eb745bd7c73458796>:0 
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.StartCreateInstance (System.String constructorSignature, System.Type declaringType, Java.Interop.JniArgumentValue* parameters) [0x0003e] in <8e80eb3ca41f4c3eb745bd7c73458796>:0 
	  at Java.Lang.Object..ctor () [0x00034] in <36a2bb1be1a64b51b22cbef15eee5437>:0 
	  ...

[0]: https://github.com/xamarin/monodroid-samples/blob/f944f42001fa134915069fa7bbb9cb493bfd446d/ExportAttribute/ExportAttributeTest/MainActivity.cs#L52-L86

The `InvalidOperationException` is thrown from
[`JNIEnv.RegisterJniNatives()` for `__export__` methods][reg], which
requires that `JavaNativeTypeManager.GetJniSignature(MethodInfo)`
return the same value as what the Java Callable Wrapper contains.

[reg]: https://github.com/xamarin/xamarin-android/blob/master/src/Mono.Android/Android.Runtime/JNIEnv.cs#L172-L178

Rephrased: we require that the Reflection-based
`JavaNativeTypeManager.GetJniSignature(MethodInfo)` generate
identical values as the Cecil-based
`JavaNativeTypeManager.GetJniSignature(MethodDefinition)`: the former
used by `JNIEnv.RegisterJniNatives()`, the latter by
`JavaCallableWrapperGenerator`. These *must* be consistent.

The problem is, they weren't:
`JavaNativeTypeManager.ToJniNameFromAttributes(Type)` -- called by
`JavaNativeTypeManager.GetJniSignature(MethodInfo)` and others -- was
erroneously updated to check for custom attributes on the specified
`Type` *and all base classes*. Checking base classes would eventually
cause `Java.Lang.Object` to be checked, causing *everything* to become
`java.lang.Object` unless explicitly overridden in the inheritance
chain (e.g. `Activity`, which has its own `[Register]` attribute).

The result is that `JavaNativeTypeManager.GetJniSignature(MethodInfo)`
for `MyParcelable.InitializeCreator()` was returning:

	()Ljava/lang/Object;

while `JavaNativeTypeManager.GetJniSignature(MethodDefinition)` was
generating:

	()Lmd5<<whatever>>/MyParcelableCreator;

Because these didn't match, `InvalidOperationException` was thrown.

Fix `JavaNativeTypeManager.ToJniNameFromAttributes(Type)` so that
base classes are *not* searched for custom attributes, restoring
prior behavior.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jan 9, 2018
Fixes: dotnet#1149

Adds support for `$(JI_MAX_JDK)` (not yet used).

Fix many, *many*, Gendarme warnings in `Java.Interop.dll`.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jan 9, 2018
jonpryor added a commit that referenced this issue Jan 9, 2018
Fixes: #1149

Adds support for `$(JI_MAX_JDK)` (not yet used).

Fix many, *many*, Gendarme warnings in `Java.Interop.dll`.
jonpryor added a commit that referenced this issue Jan 9, 2018
@fullmetaltac
Copy link
Author

Fix Verified. Issue in not reproduced.

@MP91
Copy link

MP91 commented Nov 14, 2019

Hi @ALL!
I am using Visual Studio for Mac and still have this error. I am on the latest stable release (16-3).
Is there anything I have to do?

jonpryor pushed a commit that referenced this issue Jan 23, 2021
Fixes: xamarin/monodroid#1147

Changes: http://github.com/xamarin/monodroid/compare/27736a7ffc48d606ab45598f761e873f8572f46a...daa2fb6ca52ecfd0884177da5b57501bb4dda3c6

  * xamarin/monodroid@daa2fb6ca: Bump to xamarin/xamarin-analysis@9524531 (#1149)
  * xamarin/monodroid@bca44d592: [tools/msbuild] <FastDeploy/> Length cannot be less than zero (#1148)
  * xamarin/monodroid@2df384fe2: [tools/msbuild] improve XA0010 error message (#1144)
  * xamarin/monodroid@b2da84d5f: [tools/msbuild] remove MSBuild targets related to the Xamarin Inspector (#1143)
@ghost ghost locked as resolved and limited conversation to collaborators Jun 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App Runtime Issues in `libmonodroid.so`. regression
Projects
None yet
Development

No branches or pull requests

5 participants