null reference exception in Android InAppBillingServiceConnection.OnServiceConnected()

[eliotg]

Bug Information

Please let me know if there's more info I can gather or otherwise help debug. I see a bunch of null reference checks have been added in the past month or so, but none in this method.

Version Number of Plugin: 1.2.1 (pushing new build with 1.2.2 now)
Device Tested On: Android device string: 5049W
Simulator Tested On:
Version of VS: 14.0.25431.01 Update 3
Version of Xamarin: 4.5.0.486
Versions of other things you are using: Xamarin.Android 7.3.1.2

Steps to reproduce the Behavior

Unknown exactly, I just got a crash report with the stack trace: https://gist.github.com/eliotg/5809930c07e69f412a4d0edabe07b7f6

Expected Behavior

Not crashing

Actual Behavior

Crashing

[jamesmontemagno]

#71

[erichedstrom]

I am seeing this stack trace reported occasionally too despite having MainApplication.cs in place. Currently using Plugin v. 1.2.3.107
Crashes have all happened on Android 7.0 or 7.1.1 devices, haven't reproduced locally.

[erichedstrom]

@eliotg Are you still seeing this crash reported? Or were you able to resolve it?

[eliotg]

Thanks @erichedstrom, I definitely expect so. We stopped marketing for a bit, so we're not pushing lots of people through the purchase flow, but there's no reason to suspect it isn't happening.

We put in NullReferenceException handlers wherever possible, but saw plenty of crashes outside of those because of the async nature. We're currently on 1.2.2, but we'll upgrade to 1.2.3.107 or whatever's the latest when we release a new version soon.

[erichedstrom]

I was finally able to reproduce this. It happens when the ViewModel for a Page connects to In-app Billing to load product pricing information, but then the Page pops off the Navigation stack (or is otherwise garbage collected) before the response comes and OnServiceConnected is called.

So OnServiceConnected could use some null checks on the tcsConnect and Service variables, but this error is easy enough to work around by calling ConnectAsync() from a longer-lived object.

[jamesmontemagno]

maybe we need a cancellation token in here.....
interesting.

[jamesmontemagno]

Do you want to try 1.2.4.122-beta i added more stuff in there for null checks

[singingspaniel]

There are issues with the fix in InAppBillingServiceConnection::OnServiceConnected.

After checking whether Service or Context are null, and completing the task, seems like we would want to return? Right now the code continues on using Context then Service eventhough we know that at least one of them is null.

InAppBillingServiceConnection::OnServiceConnected

[jamesmontemagno]

I have fixed this up. Thanks!