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

IProgress Callee Registration Fails on Xamarin.Android #268

Closed
StevenBonePgh opened this issue Sep 8, 2018 · 8 comments
Closed

IProgress Callee Registration Fails on Xamarin.Android #268

StevenBonePgh opened this issue Sep 8, 2018 · 8 comments

Comments

@StevenBonePgh
Copy link
Contributor

When registering a reflection-based callee with an IProgress parameter on Xamarin.Android an index out of range exception is thrown. This worked in 18.3.2, I first noticed it broken in 18.8.2, and it remains broken in 18.8.5. On .NET 4.7.1 and Xamarin.UWP the callee registers just fine. I do not have a Mac to check that platform, sorry.

The interface method likely causing the issue looks like this:

[WampProcedure("my.getfoo")]
[WampProgressiveResultProcedure]
Task<FooChunk> GetFooAsync(string fooName, IProgress<FooChunk> progress);

The exception happens when calling Channel.RealmProxy.Services.RegisterCallee with the object implementing that interface.

{System.IndexOutOfRangeException} "Index was outside the bounds of the array."
  at System.Attribute.InternalParamIsDefined (System.Reflection.ParameterInfo parameter, System.Type attributeType, System.Boolean inherit) [0x0003b] in <43dbbdc147f2482093d8409abb04c233>:0 
  at System.Attribute.IsDefined (System.Reflection.ParameterInfo element, System.Type attributeType, System.Boolean inherit) [0x00071] in <43dbbdc147f2482093d8409abb04c233>:0 
  at System.Attribute.IsDefined (System.Reflection.ParameterInfo element, System.Type attributeType) [0x00000] in <43dbbdc147f2482093d8409abb04c233>:0 
  at System.Reflection.CustomAttributeExtensions.IsDefined (System.Reflection.ParameterInfo element, System.Type attributeType) [0x00000] in <43dbbdc147f2482093d8409abb04c233>:0 
  at WampSharp.V2.Rpc.MethodInfoValidation.ValidateTupleReturnTypeOfProgressiveMethod (System.Reflection.MethodInfo method, System.Reflection.ParameterInfo lastParameter) [0x00000] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\MethodInfoValidation.cs:126 
  at WampSharp.V2.Rpc.MethodInfoValidation.ValidateProgressiveMethod (System.Reflection.MethodInfo method) [0x00081] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\MethodInfoValidation.cs:121 
  at WampSharp.V2.Rpc.OperationExtractor.CreateRpcMethod (System.Func`1[TResult] instanceProvider, WampSharp.V2.ICalleeRegistrationInterceptor interceptor, System.Reflection.MethodInfo method) [0x00040] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\OperationExtractor.cs:69 
  at WampSharp.V2.Rpc.OperationExtractor+<GetServiceMethodsOfType>d__2.MoveNext () [0x00060] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\OperationExtractor.cs:47 
  at WampSharp.V2.Rpc.OperationExtractor+<ExtractOperations>d__0.MoveNext () [0x000aa] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\OperationExtractor.cs:21 
  at WampSharp.V2.WampRealmProxyServiceProvider.RegisterCallee (System.Type type, System.Func`1[TResult] instanceProvider, WampSharp.V2.ICalleeRegistrationInterceptor interceptor) [0x00047] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Api\Client\WampRealmProxyServiceProvider.cs:70 
  at WampSharp.V2.WampRealmProxyServiceProvider.RegisterCallee (System.Object instance, WampSharp.V2.ICalleeRegistrationInterceptor interceptor) [0x0000d] in D:\a\1\s\src\net45\WampSharp\WAMP2\V2\Api\Client\WampRealmProxyServiceProvider.cs:39 

I have a CalleeRegistrationInterceptor, and right after it calls GetProcedureUri for the method in question is when the exception is thrown, which is why I suspect the above procedure is the guilty party. There seems to have been a large code churn in the areas in the call stack from when I confirmed it worked, but in a quick glance it was not obvious to me where the issue happens to be. Any ideas? I can try to nail it down further Monday if need be.

Thanks, Elad!

@darkl
Copy link
Member

darkl commented Sep 8, 2018 via email

@darkl
Copy link
Member

darkl commented Sep 8, 2018

Ok, let's look at mono's InternalParamIsDefined implementation. I guess that when the ParameterInfo is method.ReturnParameter, the parameter.Position is negative/too large, and therefore we get the IndexOutOfRangeException. I find it weird that this worked in a previous version. Maybe I changed the dependencies and it affected Xamarin.Android somehow.

Elad

@darkl
Copy link
Member

darkl commented Sep 8, 2018

So WampSharp uses ReturnParameter in a couple of more places, some run very often (the method HasMultivaluedResult). The difference between the calls is that in ValidateTupleReturnTypeOfProgressiveMethod , IsDefined is used, and in the other places GetCustomAttribute<> is used. This is probably a bug in mono, which we should file. Anyway, this can be addressed by replacing IsDefined with GetCustomAttribute<> , and checking that the attribute is not null. Usually after calling IsDefined, GetCustomAttribute<> is called, so this might make the code actually a bit shorter.

@StevenBonePgh
Copy link
Contributor Author

I created a Xamarin Android Unit test project and linked all existing WampSharp.Tests.Wampv2 tests to it. Result is 13 or 14 failures, Making the suggested changes to 4 methods where ReturnParameter.IsDefined was used and replacing it with GetCustomAttribute<> resolves all unit test failures.

src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\ResultExtractor\WampResultExtractor.cs:
    GetValueTupleResultExtractor
src\net45\WampSharp\WAMP2\V2\Rpc\Callee\Reflection\MethodInfoValidation.cs:
    ValidateTupleReturnType and ValidateTupleReturnTypeOfProgressiveMethod
src\net45\WampSharp\WAMP2\V2\Api\CalleeProxy\Callbacks\Async\OperationResultExtractor.cs:
    GetTupleArgumentUnpacker 

I created an archive with these changes and the Xamarin.Android nunit runner (I included log4net and appenders that append to the nunit log and the android logger). I included the test project in the WampSharpNetStandard.sln for my purposes. If you'd like me to add this to a PR, I'd appreciate guidance as to a proper location, but feel free to take the archive and locate it wherever you desire.

WampSharp_Resolve268.zip

I'll attempt to create a simple repro to send to Mono. To be honest, I have no idea where to report these things with Xamarin, as some are Xamarin, some are Mono, and some are Mono-[Platform].

@darkl
Copy link
Member

darkl commented Sep 8, 2018

I think you can report this bug here, see this page.

Just to clarify: the bug is with the usage of method.ReturnParameter.IsDefined.

Elad

darkl added a commit that referenced this issue Sep 8, 2018
Co-Authored-By: Steven Bone <stevenbonepgh@users.noreply.github.com>
@darkl
Copy link
Member

darkl commented Sep 8, 2018

I merged your changes, excluding the unit tests. If everything works out, v18.9.1 should be published on NuGet soon. I opened issue #269. Will be done some time in the future.

@StevenBonePgh
Copy link
Contributor Author

Just to clarify: the bug is with the usage of method.ReturnParameter.IsDefined.

Correct, that was the issue. I confirmed that 18.9.1 resolves this issue. Thanks for the assist here. It could be a recent break in Xamarin/Mono, as I also believe that I was working with an earlier version when it was working (the WampSharp version change was likely not the only change between working/nonworking).

@darkl
Copy link
Member

darkl commented Sep 11, 2018

@StevenBonePgh, can you please file a bug to mono?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants