Swrve / swrve-android-sdk

Swrve Android SDK
http://www.swrve.com
Other
10 stars 18 forks source link

ANRs in SwrveMultiLayerLocalStorage.getCombinedFirstNEvents #296

Closed gmerinojimenez closed 4 years ago

gmerinojimenez commented 4 years ago

We're having the ~2.5% of our users having ANRs due to the synchronized inside this method. Seems like a race condition, but in any case, I think it would be nice if we can avoid using synchronized in methods used by the main thread


  at com.swrve.sdk.SwrveBase.com.swrve.sdk.localstorage.SwrveMultiLayerLocalStorage.hasQueuedEvents (SourceFile:6)
                             _sendQueuedEvents
  at com.swrve.sdk.SwrveBase.sendQueuedEvents (SourceFile:6)
  at com.swrve.sdk.SwrveBase.enableEventSending (SourceFile:5)
                             switchUser
  at com.swrve.sdk.SwrveBase.identifyCachedUser (SourceFile:13)
                             _identify
  at com.swrve.sdk.SwrveBase.identify (SourceFile:4)
  at com.tuenti.swrve.EnabledSwrveWrapper.com.swrve.sdk.SwrveSDKBase.identify (SourceFile:3)
                                          setUserId
  at com.tuenti.statistics.analytics.AnalyticsUserPropertiesUpdater$initialize$2$1.accept (SourceFile:4)
                                                                                   accept
  at com.tuenti.statistics.analytics.AnalyticsUserPropertiesUpdater$initialize$2.com.annimon.stream.Optional.ifPresent (SourceFile:4)
                                                                                 com.annimon.stream.Optional.executeIfPresent
                                                                                 onUpdate
                                                                                 onUpdate
  at com.tuenti.core.dispatcher.dispatcher.UpdateDispatcher.com.tuenti.core.dispatcher.dispatcher.-$Lambda$5UxBerwlN8E4MDZd0-K-A8YaLt4.accept (SourceFile:4)
                                                            com.annimon.stream.Optional.ifPresent
                                                            subscribe
  at com.tuenti.statistics.analytics.domain.InitAnalyticsInteractor.com.tuenti.statistics.analytics.AnalyticsUserPropertiesUpdater.initialize (SourceFile:8)
                                                                    invoke
  at com.tuenti.messenger.MessengerApplication.onCreate (SourceFile:286)
  at android.app.Instrumentation.callApplicationOnCreate (Instrumentation.java:1126)
  at android.app.ActivityThread.handleBindApplication (ActivityThread.java:6062)
  at android.app.ActivityThread.-wrap1 (ActivityThread.java)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1764)
  at android.os.Handler.dispatchMessage (Handler.java:105)
  at android.os.Looper.loop (Looper.java:164)
  at android.app.ActivityThread.main (ActivityThread.java:6942)
  at java.lang.reflect.Method.invoke (Native method)
  at com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:327)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1374)```
gmerinojimenez commented 4 years ago

Update:

The other thread that is keeping the synchronized lock is this:


  at libcore.io.BlockGuardOs.poll (BlockGuardOs.java:216)
  at libcore.io.IoBridge.isConnected (IoBridge.java:254)
  at libcore.io.IoBridge.connectErrno (IoBridge.java:188)
  at libcore.io.IoBridge.connect (IoBridge.java:130)
  at java.net.PlainSocketImpl.socketConnect (PlainSocketImpl.java:129)
  at java.net.AbstractPlainSocketImpl.doConnect (AbstractPlainSocketImpl.java:356)
  at java.net.AbstractPlainSocketImpl.connectToAddress (AbstractPlainSocketImpl.java:200)
  at java.net.AbstractPlainSocketImpl.connect (AbstractPlainSocketImpl.java:182)
  at java.net.SocksSocketImpl.connect (SocksSocketImpl.java:356)
  at java.net.Socket.connect (Socket.java:616)
  at com.android.okhttp.internal.Platform.connectSocket (Platform.java:145)
  at com.android.okhttp.internal.io.RealConnection.connectSocket (RealConnection.java:1415)
  at com.android.okhttp.internal.io.RealConnection.connect (RealConnection.java:1367)
  at com.android.okhttp.internal.http.StreamAllocation.findConnection (StreamAllocation.java:219)
  at com.android.okhttp.internal.http.StreamAllocation.findHealthyConnection (StreamAllocation.java:142)
  at com.android.okhttp.internal.http.StreamAllocation.newStream (StreamAllocation.java:104)
  at com.android.okhttp.internal.http.HttpEngine.connect (HttpEngine.java:410)
  at com.android.okhttp.internal.http.HttpEngine.sendRequest (HttpEngine.java:343)
  at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute (HttpURLConnectionImpl.java:489)
  at com.android.okhttp.internal.huc.HttpURLConnectionImpl.connect (HttpURLConnectionImpl.java:131)
  at com.android.okhttp.internal.huc.DelegatingHttpsURLConnection.connect (DelegatingHttpsURLConnection.java:89)
  at com.android.okhttp.internal.huc.HttpsURLConnectionImpl.connect (HttpsURLConnectionImpl.java)
  at com.swrve.sdk.rest.RESTClient.post (SourceFile:16)
                                   post
  at com.swrve.sdk.SwrveEventsManagerImp.postBatchRequest (SourceFile:13)
                                         sendEvents
  at com.swrve.sdk.SwrveEventsManagerImp.sendStoredEvents (SourceFile:5)
  at com.swrve.sdk.SwrveBase.lambda$_sendQueuedEvents$4 (SourceFile:3)
  at com.swrve.sdk.-$$Lambda$SwrveBase$azEwN8whkbRtaMvTn7UcPZJWxkA.run (lambda)
  at com.swrve.sdk.SwrveRunnables$1.run (SourceFile:1)
  at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1162)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:636)
  at java.lang.Thread.run (Thread.java:764)```
Sergio-Mira commented 4 years ago

Hi @gmerinojimenez we are looking into this regression and will have more info soon. Thanks for the detailed info!

Sergio-Mira commented 4 years ago

We are targeting to release a hotfix this week. Will keep this issue posted.

nimeacuerdo commented 4 years ago

Hi @Sergio-Mira, we are spotting more ANRs triggered by Swrve in other flows of the application, like this one:

at com.swrve.sdk.localstorage.SwrveMultiLayerLocalStorage.getCombinedFirstNEvents (SwrveMultiLayerLocalStorage.java:108)
  at com.swrve.sdk.localstorage.SwrveMultiLayerLocalStorage.hasQueuedEvents (SwrveMultiLayerLocalStorage.java:103)
  at com.swrve.sdk.SwrveBase._sendQueuedEvents (SwrveBase.java:550)
  at com.swrve.sdk.SwrveBase.sendQueuedEvents (SwrveBase.java:1343)
  at com.swrve.sdk.SwrveBase._onResume (SwrveBase.java:613)
  at com.swrve.sdk.SwrveBase.onResume (SwrveBase.java:1378)
  at com.swrve.sdk.SwrveBase.onActivityResumed (SwrveBase.java:1968)
  at android.app.Application.dispatchActivityResumed (Application.java:455)
  at android.app.Activity.dispatchActivityResumed (Activity.java:1295)
  at android.app.Activity.onResume (Activity.java:1827)
  at androidx.fragment.app.FragmentActivity.onResume (FragmentActivity.java:456)
  at com.tuenti.commons.ui.BaseActivity.onResume (BaseActivity.java:223)
  at com.tuenti.messenger.ui.activity.MainActivity.onResume (MainActivity.java:518)
  at android.app.Instrumentation.callActivityOnResume (Instrumentation.java:1454)
  at android.app.Activity.performResume (Activity.java:8103)
  at android.app.ActivityThread.performResumeActivity (ActivityThread.java:4401)
  at android.app.ActivityThread.handleResumeActivity (ActivityThread.java:4443)
  at android.app.servertransaction.ResumeActivityItem.execute (ResumeActivityItem.java:52)
  at android.app.servertransaction.TransactionExecutor.executeLifecycleState (TransactionExecutor.java:176)
  at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:97)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2147)
  at android.os.Handler.dispatchMessage (Handler.java:107)
  at android.os.Looper.loop (Looper.java:237)
  at android.app.ActivityThread.main (ActivityThread.java:7811)
  at java.lang.reflect.Method.invoke (Method.java)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:493)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1068)

Do you have it addressed as part of that hotfix?

Sergio-Mira commented 4 years ago

Yes, this should be addressed in 7.3.2, that regression is fixed now and the lock won't be called from the UI thread. Will be closing this issue for now, let us know how it goes.

iNdieboyjeff commented 3 years ago

I'm using 7.3.2 and we're still seeing these ANRs

iNdieboyjeff commented 3 years ago

I'm using 7.3.2 and we're still seeing these ANRs

Actually ignore this. The most recent stats show that we're only getting these reports from previous app versions. The most recent app version has stopped showing this.