getsentry / sentry-xamarin

Sentry for Xamarin Native and Xamarin.Forms
https://docs.sentry.io/platforms/dotnet/guides/xamarin/
44 stars 11 forks source link

[Android] System.NullReferenceException - RequestedThemeChanged #146

Open rhaly opened 5 months ago

rhaly commented 5 months ago

Environment

Xamarin.Forms - 5.0.0.2012 Sentry.Xamarin.Forms - 1.5.2

Steps to Reproduce

After integrating sentry to our solution we started to observe random crashes at app startup with the following stacktrace:

Dictionary`2[TKey,TValue].TryInsert (TKey key, TValue value, System.Collections.Generic.InsertionBehavior behavior)
Dictionary`2[TKey,TValue].Add (TKey key, TValue value)
WeakEventManager.AddEventHandler (System.String eventName, System.Object handlerTarget, System.Reflection.MethodInfo methodInfo)
WeakEventManager.AddEventHandler[TEventArgs] (System.EventHandler`1[TEventArgs] handler, System.String eventName)
Application.add_RequestedThemeChanged (System.EventHandler`1[TEventArgs] value)
Xamarin.Forms.AppThemeBinding..ctor () <0x7a39a17e00 + 0x000ef> in <978ec34c5c9c4c4eb3d73b6da958bcd6>:0
BindingBase>.ProvideValue (System.IServiceProvider serviceProvider)
App.InitializeComponent ()
App.OnInitialized ()
PrismApplicationBase.InitializeInternal ()
Prism.PrismApplicationBase..ctor (Prism.IPlatformInitializer platformInitializer, System.Boolean setFormsDependencyResolver) <0x7a398d2580 + 0x001bf> in <fe20bbc2e107492eb7c7e45796362bb2>:0
Prism.PrismApplicationBase..ctor (Prism.IPlatformInitializer platformInitializer) <0x7a398d2550 + 0x0001f> in <fe20bbc2e107492eb7c7e45796362bb2>:0
Prism.DryIoc.PrismApplication..ctor (Prism.IPlatformInitializer platformInitializer) <0x7a398568e0 + 0x0001b> in <4aced3379c0840fcb92868797a07d195>:0
MyApp.App..ctor (Prism.IPlatformInitializer initializer) <0x7a39010a20 + 0x0001f> in <1970c5679feb497fb07e5b54b85d70b8>:0
MainActivity.OnCreate (Android.OS.Bundle bundle)
Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState)
JNINativeWrapper.Wrap_JniMarshal_PPL_V (_JniMarshal_PPL_V callback, System.IntPtr jnienv, System.IntPtr klazz, System.IntPtr p0)

Some other exceptions are also thrown with the same stacktrace:

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.

Expected Result

Sentry should not cause random crashes ;)

Actual Result

In my opinion root cause is a race condition between creating AppThemeBinding and running SentryXamarinFormsIntegration Both of those places are subscribing for RequestedThemeChanged event, XF code is doing this on calling thread, Sentry is running this on the background thread Under the hood WeakEventManager is using regular Dictionary to store list of subscribers which isn't thread safe.

Maybe at least there can be api to do this subscription for RequestedThemeChanged optional?

bruno-garcia commented 5 months ago

On that stack trace I don't see any frames with Sentry in it. Where did you spot the relationship between SentryXamarinFormsIntegration?

bruno-garcia commented 5 months ago

That said ur reasoning makes sense, @lucas-zimerman doesn't seem safe to run that code on a background thread