Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

System.ArgumentException: Handle must be valid. Parameter name: instance when calling SecureStorage.GetAsync #380

Closed
harindaka opened this issue Jul 14, 2018 · 11 comments
Labels
bug Something isn't working

Comments

@harindaka
Copy link

Description

Steps to Reproduce

Random. I have a code which makes 2 calls to SecureStorage.GetAsync in the ViewModel each time a Xamarin Forms Page loads. I'm able to crash the app by repeatedly navigating to and back from this page (repeated calls to SecureStorage.GetAsync). Most of the time it works as expected until the exception occurs

Expected Behavior

No errors

Actual Behavior

Throws the following exception at random and crashes.
Handle must be valid. Parameter name: instance

At the call to

await SecureStorage.GetAsync

{System.ArgumentException: Handle must be valid.
Parameter name: instance
  at Java.Interop.JniEnvironment+InstanceMethods.CallObjectMethod (Java.Interop.JniObjectReference instance, Java.Interop.JniMethodInfo method, Java.Interop.JniArgumentValue* args) [0x00009] in <7802aa64ad574c33adca332a3fa9706a>:0 
  at Android.Runtime.JNIEnv.CallObjectMethod (System.IntPtr jobject, System.IntPtr jmethod, Android.Runtime.JValue* parms) [0x0000e] in <263adecfa58f4c449f1ff56156d886fd>:0 
  at Android.Content.ISharedPreferencesInvoker.GetString (System.String key, System.String defValue) [0x0006c] in <263adecfa58f4c449f1ff56156d886fd>:0 
  at Xamarin.Essentials.SecureStorage.PlatformGetAsync (System.String key) [0x0001a] in <e3b2ff3137884401ace22dbf653e6c03>:0 
  at Xamarin.Essentials.SecureStorage.GetAsync (System.String key) [0x00013] in <e3b2ff3137884401ace22dbf653e6c03>:0 
  at SolutionAccelerators.Utilities.Encryption.EncryptionUtility+<ReadRijndael256KeyAsync>d__12.MoveNext () [0x0000f] in /source/SolutionAccelerators/Utilities/Encryption/EncryptionUtility.cs:173 
--- End of stack trace from previous location where exception was thrown ---
  at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <f32579baafc1404fa37ba3ec1abdc0bd>:0 
  at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <f32579baafc1404fa37ba3ec1abdc0bd>:0 
  at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <f32579baafc1404fa37ba3ec1abdc0bd>:0 
  at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <f32579baafc1404fa37ba3ec1abdc0bd>:0 
  at System.Runtime.CompilerServices.TaskAwaiter`1[TResult].GetResult () [0x00000] in <f32579baafc1404fa37ba3ec1abdc0bd>:0 
  at SolutionAccelerators.Utilities.Encryption.EncryptionUtility+<DecryptRijndael256Async>d__10.MoveNext () [0x000b9] in /source/SolutionAccelerators/Utilities/Encryption/EncryptionUtility.cs:137 }

Basic Information

  • Version with issue: 0.7.0.17-preview
  • Last known good version: ?
  • IDE: Visual Studio for Mac
  • Platform Target Frameworks: Android
    • iOS: N/A
    • Android: 8.1
    • UWP: N/A
  • Android Support Library Version:
  • Nuget Packages:
  • Affected Devices: Moto Z Play
@jamesmontemagno
Copy link
Collaborator

Can you please test with 0.8.0-preview as we made some changes there.

@harindaka
Copy link
Author

@jamesmontemagno Yes I can confirm that the issue is still in 0.8.0-preview. It's still random though. I still need to navigate back and forth to the same screen multiple times as mentioned earlier which in turn issues multiple calls to await SecureStorage.GetAsync in order to reproduce it. I pass in a non null class level private string variable as the key parameter to GetAsync if that is relevant.

@jamesmontemagno
Copy link
Collaborator

What version of Android is your device running? They run through separate paths.

Is this happening when you navigate from screen to screen and it has not completed the Get?

A small repro would be awesome is possible :)

Also did you call the Init call from https://docs.microsoft.com/en-us/xamarin/essentials/get-started?tabs=windows%2Candroid

Is this Xamarin.Forms or Xamarin.Android based app?

@jamesmontemagno jamesmontemagno added the need-more-information Need more information to investigate a bug or proposal label Jul 16, 2018
@jamesmontemagno
Copy link
Collaborator

Interesting, so this is lower down in the actual Preference class itself.... Seems like something like: https://stackoverflow.com/questions/43855538/handle-must-be-valid-exception-xamarin?rq=1

It could have something to do with calling across different screens. Investigating more.

@harindaka
Copy link
Author

@jamesmontemagno interesting indeed. Would this be related to garbage collection?

Here are the answers to your previous questions

  1. Version of Android is 8.0 (Moto Z Play)
  2. Happens when I navigate from one screen to another then go back (rinse and repeat to reproduce) SecureStorage.GetAsync gets called when the second page's viewmodel gets created. Call originates from within the viewmodel in the order viewmodel -> settingsutility -> SecureStorage.GetAsync. No direct calls to SecureStorage.GetAsync from Page 2.
  3. Will try to come up with a small project to reproduce this if I get the time
  4. Yes Init is called properly. SecureStorage.GetAsync also works most of the time. Issue is random.
  5. It's a Xamarin.Forms project

Also did you note the last comment in the link that you posted which says

Had this issue also with SharedPreferences, as soon as I tried to access it from multiple classes. SharedPreferences is a singleton, so I made my PreferencesManager class a singleton, and it resolved this issue.

@jamesmontemagno
Copy link
Collaborator

I believe that we have a fix #386 and will have a build for you to test soon once it is in master!

I believe using our own internal preference will lock correctly and prevent this edge case.

@harindaka
Copy link
Author

@jamesmontemagno Thats great! Let me know if and when I can test and confirm :)

@jamesmontemagno jamesmontemagno added this to the 0.9.0-preview milestone Jul 20, 2018
jamesmontemagno added a commit that referenced this issue Jul 20, 2018
* Use Internal Preferences for Secure Storage consistency.
Save if we created key pre-M so we always use pre-M if device upgrades.

* Fix logic for pre-m key check

The logic was slightly off, I think this fixes it, but would be good to have another set of eyes...

1. We check to see if the device is pre-M (if it does _not_ have `M`, or if we already set the fact it's pre-M in the preference - aka from a previous install before an upgrade of the OS)
2. If we aren't pre-M, we can't use Symmetric Key from Keystore
3. If we make it down to using Asymmetric Key, we set the pre-M preference to `true` to persist the value for future invocations, which will make it 'stick' in the event of a pre-M to M+ OS upgrade.

* Address feedback on key naming.

* Added test for secure storage to simulate upgrade

From API < 23 to API >= 23 after storing data with an asymmetric key and then moving to a platform supporting symmetric keys.

* Ensure we always set flags when using specified keygen
jamesmontemagno added a commit that referenced this issue Jul 31, 2018
* GH-343: Take into consideration VerticalAccuracy on iOS Altitude (#344)

* Fix for #343

Only set altitude if it's available (VerticalAccuracy is negative when it isn't according to iOS doc)

* Use: (double?)

* dotnet foundation CLA broken link (#349)

* Update ns-Xamarin.Essentials.xml

* Update README.md (#340)

* GH-365 Change to generic EventHandlers

* Clean up unit tests.

* GH-363 Enable C# 7.2 (#364)

* GH-363 Enable CD 7.3

* Update Xamarin.Essentials.csproj

* GH-367 Change Ui to UI & Ac to AC (#374)

* GH-367 Change Ui to UI

* Ensure proper Ui for Android

* Fix gryoscope

* Rename Ac to AC

* Update framework indexes

* Create Product Feedback page for Docs (#369)

* Create PRODUCT-FEEDBACK.md

* Update PRODUCT-FEEDBACK.md

* Fixed a typo from "chanaged" to "changed"

* Fixed a typo from "chanaged" to "changed"

* Fixed a typo from "chanaged" to "changed"

* GH-337 Android Low Pass Filter (#354)

* Implemented low pass filter as requested by issue 337

* Added value to sample page.

* Updated docs for ApplyLowPassFilter

* Enabled ApplyLowPassFilter Switch on Android, disabled on all other platforms

* Moved LowPassFilter to shared, removed unnecessary usings.

* Updated Docs

* Sample Project Tweaks and Optimizations (#353)

* Make sure orientation sensor stops on sample.

* Fix up all sensors page to start and stop correctly

* Only check permission if unknown status

* Update to latest sdk.extras

* fix back button press on TTS page. Check for null.

* Add samples sln for CI/CD

* Update readme and samples sln

* Don't use code sign key for simbuilds

* let's try this gain

* Update to projects

* Cleanup sample config

* bump sdk extras

* Add in readme for install with helpful information. Bump MSBuild.Sdk.Extras (#392)

* GH-368: Allow for unicode in host names and escape query (#393)

* Allow for unicode in host names and escape query

Previously we were passing the the url as is to each platform and letting it deal with how to parse into the platform specific url.

This handles unicode domain names (IDN Mapping aka PunyCode) for the domain, but also reconstructs the url based on the parsed System.Uri from the original string, and so the PathAndQuery will be the % escaped version which will account for unicode characters in it as well.

Finally, on iOS and Android we are using AbsoluteUri to end up with the fully IDN mapped and % encoded URL to pass to the system URL types.

* Extract Escape Uri logic into its own method

More testable.

* Add uri escaping tests

* Improve uri escape tests

* Update default iOS DeviceTests target

Also don’t allow picking a sim that’s unavailable (this was a bug since we were checking for contains `available` which of course `unavailable` also contains!)

* Update iOS DeviceInfo.Model to return specifid hw.machine for more control. (#395)

via:
https://github.com/xamarin/xamarin-macios/blob/bc492585d137d8c3d3a2ffc827db3cdaae3cc869/tests/linker/ios/link%20sdk/LinkSdkRegressionTest.cs#L603-L631

Added tests

* GH-380 Use Internal Preferences for Secure Storage consistency. (#386)

* Use Internal Preferences for Secure Storage consistency.
Save if we created key pre-M so we always use pre-M if device upgrades.

* Fix logic for pre-m key check

The logic was slightly off, I think this fixes it, but would be good to have another set of eyes...

1. We check to see if the device is pre-M (if it does _not_ have `M`, or if we already set the fact it's pre-M in the preference - aka from a previous install before an upgrade of the OS)
2. If we aren't pre-M, we can't use Symmetric Key from Keystore
3. If we make it down to using Asymmetric Key, we set the pre-M preference to `true` to persist the value for future invocations, which will make it 'stick' in the event of a pre-M to M+ OS upgrade.

* Address feedback on key naming.

* Added test for secure storage to simulate upgrade

From API < 23 to API >= 23 after storing data with an asymmetric key and then moving to a platform supporting symmetric keys.

* Ensure we always set flags when using specified keygen

* GH-388 If activity is required and null throw null exception with information. (#396)

* If activity is required and null throw null exception with information.

* Remove currentactivity and update getcurrentactivity.

* GH-250 Implement Launcher API (#356) (#405)

* GH-250 Implement Launcher API (#356)

* Initial commit for Launcher Api as discussed in Issue 250

* Fixed and refactored android launch code, fixed unit tests. Fixed namespaces.

* Refactored code, fixed uwp launcher. Added unit test

* Added sample launcher page and viewmodel.

* Fixed launcher Page typo

* Refactored launcher code. after testing exception behaviour I decided on rethrowing instead of swallowing the exception

* Refactored access modifiers. Also changed exception behaviour to enforce uri validity and exception behaviour across platforms.

* Updated docs, stripped even more code.

* Added validation. Adressed all other changes as required in pr

* Removed dead code

* Adressed last issues from pr

* Cleanup launcher

* cleanup iOS tests

* Early morning typos

* Name Alignment for BrowserLaunchMode (#408)

* Rename BrowserLaunchType to BrowserLaunchMode

* update nuget

* Rename parameter

* GH-287 OpenMaps Implementation (#361) (#404)

* GH-287 Maps Implementation (#361)

* Implemented maps as mentioned in branch name issue. also fixed a spelling mistake

* Added samples

* Added doc stubs

* Added comments for map types

* Refactored code

* Formatting of docs

* Added tests

* Changed coordinates to display something recognisable

* Added uri escaping, thus removing duplicate code. also had to turn off a warning due to inconsistent style cop and vs warning behaviour.

* Removed ref until in is added in c# 7.2

* Adressed issues in pr

* Updated launch code

* Adressed method to onliner

* Updated docs

* Removed ClearTop intent, added sample with adress

* Rename to align names. Added extensions and additional paramaters.

* Update tests

* Throw if options are null.

* Add overload for lat/long without options

* Enable multi-target for 25,26,27 (#411)

Alight nuget versions to xamarin.forms.

* No More Shared Projects (#410)

* Change to multi-targeted project for device tests :)

* Cleanup scripts

* Fix build error due to preprocessor directive (#417)

* Sync Sensor Speeds & Reset UWP to 0 (#419)

* Restore the default report interval to release resources while the sensor is not in use

* Cleanup sensor reading times.
Make them all the same and only in 1 place
Compass is still unique

* Create unique preference storage for each feature. (#434)

* Create unique preference storage for each feature.

* update storage per @Redth

* Update version
@jamesmontemagno
Copy link
Collaborator

0.9.0 should have it in there :) available righ tnow :)

@jamesmontemagno jamesmontemagno removed this from the 0.10.0-preview milestone Aug 2, 2018
@jamesmontemagno jamesmontemagno added bug Something isn't working and removed need-more-information Need more information to investigate a bug or proposal labels Aug 2, 2018
@harindaka
Copy link
Author

@jamesmontemagno I can confirm that 0.9.0 seems to have fixed this issue. I can no longer reproduce it. Thanks!

@jamesmontemagno
Copy link
Collaborator

Awesome! Thanks you for testing it out :)

@KurasovAnton
Copy link

KurasovAnton commented Feb 12, 2021

Hi @jamesmontemagno , we still receiving the logs with this defect

Xam.Plugin.SecureStorage 1.0.13
Devices:
Pixel 2 - Android 11
Samsung galaxy s5 - Android 6.0.1

Issue:
SettingsImplementation.AddOrUpdateValueCore (System.String key, System.Object value, System.TypeCode typeCode, System.String fileName)
System.ArgumentException: Handle must be valid. Parameter name: type

Stack trace:

Java.Interop
JniEnvironment+InstanceMethods.GetMethodID (Java.Interop.JniObjectReference type, System.String name, System.String signature)
Android.Runtime
JNIEnv.GetMethodID (System.IntPtr kls, System.String name, System.String signature)
Android.Content
ISharedPreferencesInvoker.Edit ()
Plugin.Settings
SettingsImplementation.AddOrUpdateValueCore (System.String key, System.Object value, System.TypeCode typeCode, System.String fileName)
Plugin.Settings
SettingsImplementation.AddOrUpdateValueInternal[T] (System.String key, T value, System.String fileName)
Plugin.Settings
SettingsImplementation.AddOrUpdateValue (System.String key, System.String value, System.String fileName)

Also, the calls to the ISettingStorage SetValue and GetValue methods, wrapped with lock statements

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants