Fix crash(es) caused by failing to dispose Rx observables #2669

dbrant · 2019-03-19T14:16:20Z

You have numerous usages of Rx observables that are not properly contained in a Disposable object, and are not disposed when you leave the current activity or fragment. This can lead to all manner of crashes and subtle bugs.

...to be used by subclasses, and gets disposed automatically.

codecov-io · 2019-03-19T14:31:18Z

Codecov Report

Merging #2669 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #2669      +/-   ##
=========================================
- Coverage    2.77%   2.77%   -0.01%     
=========================================
  Files         261     261              
  Lines       12453   12465      +12     
  Branches     1130    1128       -2     
=========================================
  Hits          346     346              
- Misses      12080   12092      +12     
  Partials       27      27

Impacted Files	Coverage Δ
...n/java/fr/free/nrw/commons/theme/BaseActivity.java	`0% <0%> (ø)`	⬆️
...ava/fr/free/nrw/commons/nearby/NearbyFragment.java	`0% <0%> (ø)`	⬆️
...ons/explore/categories/SearchCategoryFragment.java	`0% <0%> (ø)`	⬆️
...r/free/nrw/commons/contributions/MainActivity.java	`0% <0%> (ø)`	⬆️
...fr/free/nrw/commons/media/MediaDetailFragment.java	`0% <0%> (ø)`	⬆️
...ava/fr/free/nrw/commons/upload/UploadActivity.java	`0% <0%> (ø)`	⬆️
...w/commons/contributions/ContributionsFragment.java	`0% <0%> (ø)`	⬆️
...r/free/nrw/commons/auth/AuthenticatedActivity.java	`0% <0%> (ø)`	⬆️
...rw/commons/explore/images/SearchImageFragment.java	`0% <0%> (ø)`	⬆️
...w/commons/category/CategoryImagesListFragment.java	`0% <0%> (ø)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3417d71...d1c83a6. Read the comment docs.

domdomegg

Tested 2.10.1-debug-RxFixes2~d1c83a619

Beta Commons Upload

As I was testing I got quite a few crashes when navigating quickly between activities. I think this happens in master although it felt more common testing this PR. I think #2668 might fix these and I'm unsure whether this PR makes this issue worse I'm going to wait for that to be merged first.

2019-03-19 20:12:42.780 3811-3811/fr.free.nrw.commons.beta E/ContributionsFragment: onFragmentResumed fr.free.nrw.commons.contributions.ContributionsListFragment
2019-03-19 20:12:42.995 3811-3811/fr.free.nrw.commons.beta E/ContributionsFragment: onFragmentDetached fr.free.nrw.commons.contributions.ContributionsListFragment
2019-03-19 20:12:42.996 3811-3863/fr.free.nrw.commons.beta E/ACRA: ACRA caught a UndeliverableException for fr.free.nrw.commons.beta
    io.reactivex.exceptions.UndeliverableException: The exception could not be delivered to the consumer because it has already canceled/disposed the flow or the exception has nowhere to go to begin with. Further reading: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#error-handling | java.io.InterruptedIOException
        at io.reactivex.plugins.RxJavaPlugins.onError(RxJavaPlugins.java:367)
        at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:50)
        at io.reactivex.Single.subscribe(Single.java:3438)
        at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89)
        at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:578)
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66)
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)
     Caused by: java.io.InterruptedIOException
        at okhttp3.internal.http2.Http2Stream.waitForIo(Http2Stream.java:642)
        at okhttp3.internal.http2.Http2Stream.takeHeaders(Http2Stream.java:150)
        at okhttp3.internal.http2.Http2Codec.readResponseHeaders(Http2Codec.java:131)
        at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:88)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:45)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:126)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.java:213)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:254)
        at okhttp3.RealCall.execute(RealCall.java:92)
        at fr.free.nrw.commons.mwapi.OkHttpJsonApiClient.lambda$getAchievements$2(OkHttpJsonApiClient.java:129)
        at fr.free.nrw.commons.mwapi.-$$Lambda$OkHttpJsonApiClient$e1yuTHfYTkBd47P3BZbbQnvFIto.call(Unknown Source:6)
        at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:44)
        at io.reactivex.Single.subscribe(Single.java:3438) 
        at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89) 
        at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:578) 
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66) 
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57) 
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301) 
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:764) 
2019-03-19 20:12:43.002 3811-3876/fr.free.nrw.commons.beta E/ACRA: ACRA caught a UndeliverableException for fr.free.nrw.commons.beta
    io.reactivex.exceptions.UndeliverableException: The exception could not be delivered to the consumer because it has already canceled/disposed the flow or the exception has nowhere to go to begin with. Further reading: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#error-handling | java.io.InterruptedIOException
        at io.reactivex.plugins.RxJavaPlugins.onError(RxJavaPlugins.java:367)
        at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:50)
        at io.reactivex.Single.subscribe(Single.java:3438)
        at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89)
        at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:578)
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66)
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)
     Caused by: java.io.InterruptedIOException
        at okhttp3.internal.http2.Http2Stream.waitForIo(Http2Stream.java:642)
        at okhttp3.internal.http2.Http2Stream.takeHeaders(Http2Stream.java:150)
        at okhttp3.internal.http2.Http2Codec.readResponseHeaders(Http2Codec.java:131)
        at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:88)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:45)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:126)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.java:213)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:254)
        at okhttp3.RealCall.execute(RealCall.java:92)
        at fr.free.nrw.commons.mwapi.OkHttpJsonApiClient.lambda$getUploadCount$0(OkHttpJsonApiClient.java:74)
        at fr.free.nrw.commons.mwapi.-$$Lambda$OkHttpJsonApiClient$9ZpGX0U5Zngts_jcRCFls7FtEIM.call(Unknown Source:4)
        at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:44)
        at io.reactivex.Single.subscribe(Single.java:3438) 
        at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89) 
        at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:578) 
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66) 
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57) 
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301) 
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:764) 
2019-03-19 20:12:43.079 2713-739/com.google.android.googlequicksearchbox:search E/EntrySyncManager: Cannot determine account name: drop request
2019-03-19 20:12:43.079 2713-739/com.google.android.googlequicksearchbox:search E/NowController: Failed to access data from EntryProvider. ExecutionException.
    java.util.concurrent.ExecutionException: com.google.android.apps.gsa.sidekick.main.h.n: Could not complete scheduled request to refresh entries. ClientErrorCode: 3
        at com.google.common.util.concurrent.d.eA(SourceFile:85)
        at com.google.common.util.concurrent.d.get(SourceFile:23)
        at com.google.common.util.concurrent.l.get(SourceFile:2)
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.be.cbB(SourceFile:49)
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.be.cbA(SourceFile:181)
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.bh.run(Unknown Source:2)
        at com.google.android.apps.gsa.shared.util.concurrent.at.run(SourceFile:4)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:458)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:458)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at com.google.android.apps.gsa.shared.util.concurrent.b.g.run(Unknown Source:4)
        at com.google.android.apps.gsa.shared.util.concurrent.b.aw.run(SourceFile:4)
        at com.google.android.apps.gsa.shared.util.concurrent.b.aw.run(SourceFile:4)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)
        at com.google.android.apps.gsa.shared.util.concurrent.b.i.run(SourceFile:6)
     Caused by: com.google.android.apps.gsa.sidekick.main.h.n: Could not complete scheduled request to refresh entries. ClientErrorCode: 3
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.ar.az(Unknown Source:4)
        at com.google.common.util.concurrent.q.ap(SourceFile:7)
        at com.google.common.util.concurrent.p.run(SourceFile:32)
        at com.google.common.util.concurrent.bt.execute(SourceFile:3)
        at com.google.common.util.concurrent.d.b(SourceFile:275)
        at com.google.common.util.concurrent.d.addListener(SourceFile:135)
        at com.google.common.util.concurrent.p.b(SourceFile:3)
        at com.google.android.apps.gsa.shared.util.concurrent.h.a(SourceFile:16)
        at com.google.android.apps.gsa.shared.util.concurrent.h.a(SourceFile:13)
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.be.cbB(SourceFile:47)
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.be.cbA(SourceFile:181) 
        at com.google.android.apps.gsa.staticplugins.nowstream.b.a.bh.run(Unknown Source:2) 
        at com.google.android.apps.gsa.shared.util.concurrent.at.run(SourceFile:4) 
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:458) 
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:458) 
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
        at com.google.android.apps.gsa.shared.util.concurrent.b.g.run(Unknown Source:4) 
        at com.google.android.apps.gsa.shared.util.concurrent.b.aw.run(SourceFile:4) 
        at com.google.android.apps.gsa.shared.util.concurrent.b.aw.run(SourceFile:4) 
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:764) 
        at com.google.android.apps.gsa.shared.util.concurrent.b.i.run(SourceFile:6) 
2019-03-19 20:12:43.126 3811-3811/fr.free.nrw.commons.beta E/ContributionsFragment: onFragmentDetached fr.free.nrw.commons.contributions.ContributionsListFragment
2019-03-19 20:12:51.231 1961-2094/system_process E/SyncManager: Job params not found for 106441

dbrant · 2019-03-19T20:19:13Z

Ah yes, good observation. I should have noted that this PR is loosely "dependent" on #2668, since the proper disposal of Observables can cause them to dispatch an error (i.e. a network call being cancelled), and if the error is not handled properly, it will crash.

...to be used by subclasses, and gets disposed automatically.

…o RxFixes2

dbrant · 2019-03-19T20:38:39Z

@domdomegg I've gone ahead and consolidated the other two Rx-related patches into this one. Not sure why I didn't just do that from the beginning. Hopefully this is now simpler to test.

domdomegg

Tested 2.10.1-debug-RxFixes2~14a2975ea

Beta Commons Upload

dbrant added 11 commits March 19, 2019 09:18

Add missing global error handler.

3657487

Remove unnecessary disposable container.

4d0db8d

Add missing disposal logic.

d6e630f

Add convenience CompoundDisposable to BaseActivity.

df37a07

...to be used by subclasses, and gets disposed automatically.

Add missing disposable logic.

78c85c4

Add missing dispose logic.

b1ad7a2

Add missing dispose logic.

9033acc

Fix additional missing Rx disposal logic.

e1fbb02

Fix even more Rx dispose logic.

d4f3fa7

Simplify dispose logic.

d6ab30f

Remove redundant CompositeDisposable.

d1c83a6

Properly contain and dispose of additional observables.

c2a20ce

dbrant requested review from misaochan and maskaravivek March 19, 2019 16:37

domdomegg self-requested a review March 19, 2019 19:45

domdomegg reviewed Mar 19, 2019

View reviewed changes

dbrant added 12 commits March 19, 2019 16:31

Remove unnecessary disposable container.

5ab731d

Add missing disposal logic.

69b4326

Add convenience CompoundDisposable to BaseActivity.

0e261d8

...to be used by subclasses, and gets disposed automatically.

Add missing disposable logic.

f15972c

Add missing dispose logic.

078234b

Add missing dispose logic.

f812f37

Fix additional missing Rx disposal logic.

90a0548

Fix even more Rx dispose logic.

94bf47b

Simplify dispose logic.

51e119e

Remove redundant CompositeDisposable.

3c8dc09

Merge branch 'RxFixes2' of github.com:dbrant/apps-android-commons int…

cfeed69

…o RxFixes2

Merge branch 'RxFixes3' into RxFixes2

14a2975

This was referenced Mar 19, 2019

Fix crash(es) caused by disposed Rx observables. #2668

Closed

Prevent additional possible crash(es) from improperly disposed observables. #2670

Closed

domdomegg self-requested a review March 19, 2019 21:04

domdomegg approved these changes Mar 19, 2019

View reviewed changes

domdomegg merged commit 8474c04 into commons-app:master Mar 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix crash(es) caused by failing to dispose Rx observables #2669

Fix crash(es) caused by failing to dispose Rx observables #2669

dbrant commented Mar 19, 2019

codecov-io commented Mar 19, 2019

domdomegg left a comment •

edited

Loading

dbrant commented Mar 19, 2019

dbrant commented Mar 19, 2019

domdomegg left a comment

Fix crash(es) caused by failing to dispose Rx observables #2669

Fix crash(es) caused by failing to dispose Rx observables #2669

Conversation

dbrant commented Mar 19, 2019

codecov-io commented Mar 19, 2019

Codecov Report

domdomegg left a comment • edited Loading

Choose a reason for hiding this comment

dbrant commented Mar 19, 2019

dbrant commented Mar 19, 2019

domdomegg left a comment

Choose a reason for hiding this comment

domdomegg left a comment •

edited

Loading