getsentry / sentry-react-native

Official Sentry SDK for React Native
https://sentry.io
MIT License
1.58k stars 337 forks source link

[Android] ConcurrentModification exception for frameMetricsAggregator #1865

Closed jayshah-swiggy closed 2 years ago

jayshah-swiggy commented 3 years ago

Environment

How do you use Sentry? sentry.io

Which SDK and version? Sentry react native. 3.0.0

Steps to Reproduce

Some of the app launches show following stack trace:

java.util.ConcurrentModificationException: null
    at java.util.ArrayList$Itr.next(ArrayList.java:860)
    at android.view.View.registerPendingFrameMetricsObservers(View.java:6363)
    at android.view.View.dispatchAttachedToWindow(View.java:17755)
    at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3417)
    at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1932)
    at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1647)
    at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:7595)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:988)
    at android.view.Choreographer.doCallbacks(Choreographer.java:800)
    at android.view.Choreographer.doFrame(Choreographer.java:713)
    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:974)
    at android.os.Handler.handleCallback(Handler.java:790)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:6977)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:528)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:910)

Expected Result

No error

Here is my reasoning: RNSentryModule is the only package registering frameMetricsAggregator.

I think the temp fix would be to disable perf tracking via turning off enableAutoPerformanceTracking, but this seems like something worth looking into.

Actual Result

java.util.ConcurrentModificationException: null
    at java.util.ArrayList$Itr.next(ArrayList.java:860)
    at android.view.View.registerPendingFrameMetricsObservers(View.java:6363)
    at android.view.View.dispatchAttachedToWindow(View.java:17755)
    at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:3417)
    at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1932)
    at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1647)
    at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:7595)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:988)
    at android.view.Choreographer.doCallbacks(Choreographer.java:800)
    at android.view.Choreographer.doFrame(Choreographer.java:713)
    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:974)
    at android.os.Handler.handleCallback(Handler.java:790)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:6977)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:528)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:910)
marandaneto commented 3 years ago

@jayshah-swiggy I don't see Sentry frames in the stack trace, I also don't see how frameMetricsAggregator can throw ConcurrentModificationException in our code, can you provide more context? how to reproduce? maybe even a minimal reproducible example on GH? thanks

jayshah-swiggy commented 3 years ago

@marandaneto Sentry is adding framemetrics listener via https://github.com/getsentry/sentry-react-native/blob/master/android/src/main/java/io/sentry/react/RNSentryModule.java#L136

Which uses following api internally: https://developer.android.com/reference/android/view/Window?hl=en#addOnFrameMetricsAvailableListener(android.view.Window.OnFrameMetricsAvailableListener,%20android.os.Handler)

My assumption is this should probably be posted to main activity thread, currently it seems to be happening from native_modules thread. what you see in stack trace are common framework callbacks that happen on main thread e.g. performTraversals

common arraylist they refer could be found here: http://androidxref.com/9.0.0_r3/xref/frameworks/base/core/java/android/view/View.java#6467

Sentry is the only code source dealing with frame metrics in our app.

marandaneto commented 3 years ago

@jayshah-swiggy thanks for replying, removing Sentry does the crash stop? I can't reproduce it using the latest versions, have you tried upgrading the Sentry RN SDK?

Please, set up a minimal reproducible example on GH, thanks

jayshah-swiggy commented 3 years ago

We will do a release with disabled auto perf tracking and see if issue subsides. will let you know.

marandaneto commented 3 years ago

@jayshah-swiggy while disabling auto perf tracking is a workaround for you, we'd really appreciate a minimal reproducible example, so we could debug and solve it if it's a Sentry issue, thanks.

jayshah-swiggy commented 3 years ago

Since it is a race condition, it is hard to reporduce, we get about 1 events for every 1k users. More over releasing with disabled auto perf tracking, fixed the issue for us. Zero issues since this disabling. Requesting you to investigate the same.

marandaneto commented 3 years ago

thanks @jayshah-swiggy we will have a look