Skip to content

Commit 8474c04

Browse files
dbrantdomdomegg
dbrant
authored and
domdomegg
committed
Fix crash(es) caused by failing to dispose Rx observables (#2669)
1 parent 38d39e0 commit 8474c04

21 files changed

+121
-86
lines changed

‎app/src/main/java/fr/free/nrw/commons/CommonsApplication.java

+5
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
import fr.free.nrw.commons.upload.FileUtils;
4545
import fr.free.nrw.commons.utils.ConfigUtils;
4646
import io.reactivex.android.schedulers.AndroidSchedulers;
47+
import io.reactivex.internal.functions.Functions;
48+
import io.reactivex.plugins.RxJavaPlugins;
4749
import io.reactivex.schedulers.Schedulers;
4850
import timber.log.Timber;
4951

@@ -128,6 +130,9 @@ public void onCreate() {
128130

129131
createNotificationChannel(this);
130132

133+
// This handler will catch exceptions thrown from Observables after they are disposed,
134+
// or from Observables that are (deliberately or not) missing an onError handler.
135+
RxJavaPlugins.setErrorHandler(Functions.emptyConsumer());
131136

132137
if (setupLeakCanary() == RefWatcher.DISABLED) {
133138
return;

‎app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,12 @@ protected void onCreate(Bundle savedInstanceState) {
142142
initDrawer();
143143
}
144144

145+
@Override
146+
public void onDestroy() {
147+
super.onDestroy();
148+
compositeDisposable.clear();
149+
}
150+
145151
/**
146152
* To invoke the AlertDialog on clicking info button
147153
*/
@@ -238,12 +244,12 @@ private void setWikidataEditCount() {
238244
if (StringUtils.isNullOrWhiteSpace(userName)) {
239245
return;
240246
}
241-
okHttpJsonApiClient.getWikidataEdits(userName)
247+
compositeDisposable.add(okHttpJsonApiClient.getWikidataEdits(userName)
242248
.subscribeOn(Schedulers.io())
243249
.observeOn(AndroidSchedulers.mainThread())
244250
.subscribe(edits -> wikidataEditsText.setText(String.valueOf(edits)), e -> {
245251
Timber.e("Error:" + e);
246-
});
252+
}));
247253
}
248254

249255
private void showSnackBarWithRetry() {

‎app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,12 @@ protected void onSaveInstanceState(Bundle outState) {
5858
* Makes API call to check if user is blocked from Commons. If the user is blocked, a snackbar
5959
* is created to notify the user
6060
*/
61-
protected void showBlockStatus()
62-
{
63-
Observable.fromCallable(() -> mediaWikiApi.isUserBlockedFromCommons())
61+
protected void showBlockStatus() {
62+
compositeDisposable.add(Observable.fromCallable(() -> mediaWikiApi.isUserBlockedFromCommons())
6463
.subscribeOn(Schedulers.io())
6564
.observeOn(AndroidSchedulers.mainThread())
6665
.filter(result -> result)
6766
.subscribe(result -> ViewUtil.showShortSnackbar(findViewById(android.R.id.content), R.string.block_notification)
68-
);
67+
));
6968
}
7069
}

‎app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import fr.free.nrw.commons.utils.ViewUtil;
5353
import io.reactivex.Observable;
5454
import io.reactivex.android.schedulers.AndroidSchedulers;
55+
import io.reactivex.disposables.CompositeDisposable;
5556
import io.reactivex.schedulers.Schedulers;
5657
import timber.log.Timber;
5758

@@ -83,6 +84,7 @@ public class LoginActivity extends AccountAuthenticatorActivity {
8384
ProgressDialog progressDialog;
8485
private AppCompatDelegate delegate;
8586
private LoginTextWatcher textWatcher = new LoginTextWatcher();
87+
private CompositeDisposable compositeDisposable = new CompositeDisposable();
8688

8789
private Boolean loginCurrentlyInProgress = false;
8890
private Boolean errorMessageShown = false;
@@ -195,6 +197,7 @@ protected void onResume() {
195197

196198
@Override
197199
protected void onDestroy() {
200+
compositeDisposable.clear();
198201
try {
199202
// To prevent leaked window when finish() is called, see http://stackoverflow.com/questions/32065854/activity-has-leaked-window-at-alertdialog-show-method
200203
if (progressDialog != null && progressDialog.isShowing()) {
@@ -219,10 +222,10 @@ private void performLogin() {
219222
String twoFactorCode = twoFactorEdit.getText().toString();
220223

221224
showLoggingProgressBar();
222-
Observable.fromCallable(() -> login(username, password, twoFactorCode))
225+
compositeDisposable.add(Observable.fromCallable(() -> login(username, password, twoFactorCode))
223226
.subscribeOn(Schedulers.io())
224227
.observeOn(AndroidSchedulers.mainThread())
225-
.subscribe(result -> handleLogin(username, rawUsername, password, result));
228+
.subscribe(result -> handleLogin(username, rawUsername, password, result)));
226229
}
227230

228231
private String login(String username, String password, String twoFactorCode) {

‎app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesFragment.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import fr.free.nrw.commons.utils.ViewUtil;
3131
import io.reactivex.Observable;
3232
import io.reactivex.android.schedulers.AndroidSchedulers;
33+
import io.reactivex.disposables.CompositeDisposable;
3334
import io.reactivex.schedulers.Schedulers;
3435
import timber.log.Timber;
3536

@@ -41,6 +42,7 @@ public class BookmarkPicturesFragment extends DaggerFragment {
4142
private static final int TIMEOUT_SECONDS = 15;
4243

4344
private GridViewAdapter gridAdapter;
45+
private CompositeDisposable compositeDisposable = new CompositeDisposable();
4446

4547
@BindView(R.id.statusMessage) TextView statusTextView;
4648
@BindView(R.id.loadingImagesProgressBar) ProgressBar progressBar;
@@ -82,6 +84,12 @@ public void onStop() {
8284
controller.stop();
8385
}
8486

87+
@Override
88+
public void onDestroy() {
89+
super.onDestroy();
90+
compositeDisposable.clear();
91+
}
92+
8593
@Override
8694
public void onResume() {
8795
super.onResume();
@@ -113,11 +121,11 @@ private void initList() {
113121
progressBar.setVisibility(VISIBLE);
114122
statusTextView.setVisibility(GONE);
115123

116-
Observable.fromCallable(() -> controller.loadBookmarkedPictures())
124+
compositeDisposable.add(Observable.fromCallable(() -> controller.loadBookmarkedPictures())
117125
.subscribeOn(Schedulers.io())
118126
.observeOn(AndroidSchedulers.mainThread())
119127
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
120-
.subscribe(this::handleSuccess, this::handleError);
128+
.subscribe(this::handleSuccess, this::handleError));
121129
}
122130

123131
/**

‎app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java

+12-4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import fr.free.nrw.commons.utils.ViewUtil;
3232
import io.reactivex.Observable;
3333
import io.reactivex.android.schedulers.AndroidSchedulers;
34+
import io.reactivex.disposables.CompositeDisposable;
3435
import io.reactivex.schedulers.Schedulers;
3536
import timber.log.Timber;
3637

@@ -51,6 +52,7 @@ public class CategoryImagesListFragment extends DaggerFragment {
5152
@BindView(R.id.loadingImagesProgressBar) ProgressBar progressBar;
5253
@BindView(R.id.categoryImagesList) GridView gridView;
5354
@BindView(R.id.parentLayout) RelativeLayout parentLayout;
55+
private CompositeDisposable compositeDisposable = new CompositeDisposable();
5456
private boolean hasMoreImages = true;
5557
private boolean isLoading = true;
5658
private String categoryName = null;
@@ -74,6 +76,12 @@ public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
7476
initViews();
7577
}
7678

79+
@Override
80+
public void onDestroy() {
81+
super.onDestroy();
82+
compositeDisposable.clear();
83+
}
84+
7785
/**
7886
* Initializes the UI elements for the fragment
7987
* Setup the grid view to and scroll listener for it
@@ -109,11 +117,11 @@ private void initList() {
109117

110118
isLoading = true;
111119
progressBar.setVisibility(VISIBLE);
112-
Observable.fromCallable(() -> controller.getCategoryImages(categoryName))
120+
compositeDisposable.add(Observable.fromCallable(() -> controller.getCategoryImages(categoryName))
113121
.subscribeOn(Schedulers.io())
114122
.observeOn(AndroidSchedulers.mainThread())
115123
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
116-
.subscribe(this::handleSuccess, this::handleError);
124+
.subscribe(this::handleSuccess, this::handleError));
117125
}
118126

119127
/**
@@ -215,11 +223,11 @@ private void fetchMoreImages() {
215223
}
216224

217225
progressBar.setVisibility(VISIBLE);
218-
Observable.fromCallable(() -> controller.getCategoryImages(categoryName))
226+
compositeDisposable.add(Observable.fromCallable(() -> controller.getCategoryImages(categoryName))
219227
.subscribeOn(Schedulers.io())
220228
.observeOn(AndroidSchedulers.mainThread())
221229
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
222-
.subscribe(this::handleSuccess, this::handleError);
230+
.subscribe(this::handleSuccess, this::handleError));
223231
}
224232

225233
/**

‎app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -97,17 +97,17 @@ public void initSubCategoryList() {
9797
}
9898
progressBar.setVisibility(View.VISIBLE);
9999
if (!isParentCategory){
100-
Observable.fromCallable(() -> mwApi.getSubCategoryList(categoryName))
100+
compositeDisposable.add(Observable.fromCallable(() -> mwApi.getSubCategoryList(categoryName))
101101
.subscribeOn(Schedulers.io())
102102
.observeOn(AndroidSchedulers.mainThread())
103103
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
104-
.subscribe(this::handleSuccess, this::handleError);
104+
.subscribe(this::handleSuccess, this::handleError));
105105
}else {
106-
Observable.fromCallable(() -> mwApi.getParentCategoryList(categoryName))
106+
compositeDisposable.add(Observable.fromCallable(() -> mwApi.getParentCategoryList(categoryName))
107107
.subscribeOn(Schedulers.io())
108108
.observeOn(AndroidSchedulers.mainThread())
109109
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
110-
.subscribe(this::handleSuccess, this::handleError);
110+
.subscribe(this::handleSuccess, this::handleError));
111111
}
112112
}
113113

‎app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java

+2-8
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import io.reactivex.Observable;
6262
import io.reactivex.android.schedulers.AndroidSchedulers;
6363
import io.reactivex.disposables.CompositeDisposable;
64-
import io.reactivex.disposables.Disposable;
6564
import io.reactivex.schedulers.Schedulers;
6665
import timber.log.Timber;
6766

@@ -101,7 +100,6 @@ public class ContributionsFragment
101100
@BindView(R.id.card_view_nearby) public NearbyNotificationCardView nearbyNotificationCardView;
102101
@BindView(R.id.campaigns_view) CampaignView campaignView;
103102

104-
private Disposable placesDisposable;
105103
private LatLng curLatLng;
106104

107105
private boolean firstLocationUpdate = true;
@@ -592,15 +590,15 @@ private void displayYouWontSeeNearbyMessage() {
592590
private void updateClosestNearbyCardViewInfo() {
593591
curLatLng = locationManager.getLastLocation();
594592

595-
placesDisposable = Observable.fromCallable(() -> nearbyController
593+
compositeDisposable.add(Observable.fromCallable(() -> nearbyController
596594
.loadAttractionsFromLocation(curLatLng, curLatLng, true, false)) // thanks to boolean, it will only return closest result
597595
.subscribeOn(Schedulers.io())
598596
.observeOn(AndroidSchedulers.mainThread())
599597
.subscribe(this::updateNearbyNotification,
600598
throwable -> {
601599
Timber.d(throwable);
602600
updateNearbyNotification(null);
603-
});
601+
}));
604602
}
605603

606604
private void updateNearbyNotification(@Nullable NearbyController.NearbyPlacesInfo nearbyPlacesInfo) {
@@ -635,10 +633,6 @@ public void onDestroy() {
635633
isUploadServiceConnected = false;
636634
}
637635
}
638-
639-
if (placesDisposable != null) {
640-
placesDisposable.dispose();
641-
}
642636
}
643637

644638
@Override

‎app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,11 @@ public boolean onCreateOptionsMenu(Menu menu) {
307307

308308
@SuppressLint("CheckResult")
309309
private void setNotificationCount() {
310-
Observable.fromCallable(() -> notificationController.getNotifications(false))
310+
compositeDisposable.add(Observable.fromCallable(() -> notificationController.getNotifications(false))
311311
.subscribeOn(Schedulers.io())
312312
.observeOn(AndroidSchedulers.mainThread())
313313
.subscribe(this::initNotificationViews,
314-
throwable -> Timber.e(throwable, "Error occurred while loading notifications"));
314+
throwable -> Timber.e(throwable, "Error occurred while loading notifications")));
315315
}
316316

317317
private void initNotificationViews(List<Notification> notificationList) {

‎app/src/main/java/fr/free/nrw/commons/di/CommonsDaggerSupportFragment.java

+9
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,27 @@
99
import dagger.android.AndroidInjector;
1010
import dagger.android.DispatchingAndroidInjector;
1111
import dagger.android.support.HasSupportFragmentInjector;
12+
import io.reactivex.disposables.CompositeDisposable;
1213

1314
public abstract class CommonsDaggerSupportFragment extends Fragment implements HasSupportFragmentInjector {
1415

1516
@Inject
1617
DispatchingAndroidInjector<Fragment> childFragmentInjector;
1718

19+
protected CompositeDisposable compositeDisposable = new CompositeDisposable();
20+
1821
@Override
1922
public void onAttach(Context context) {
2023
inject();
2124
super.onAttach(context);
2225
}
2326

27+
@Override
28+
public void onDestroy() {
29+
super.onDestroy();
30+
compositeDisposable.clear();
31+
}
32+
2433
@Override
2534
public AndroidInjector<Fragment> supportFragmentInjector() {
2635
return childFragmentInjector;

‎app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void setTabs() {
9898

9999
viewPagerAdapter.setTabData(fragmentList, titleList);
100100
viewPagerAdapter.notifyDataSetChanged();
101-
RxSearchView.queryTextChanges(searchView)
101+
compositeDisposable.add(RxSearchView.queryTextChanges(searchView)
102102
.takeUntil(RxView.detaches(searchView))
103103
.debounce(500, TimeUnit.MILLISECONDS)
104104
.observeOn(AndroidSchedulers.mainThread())
@@ -120,7 +120,7 @@ public void setTabs() {
120120
searchHistoryContainer.setVisibility(View.VISIBLE);
121121
}
122122
}
123-
);
123+
));
124124
}
125125

126126
/**

‎app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
import android.content.res.Configuration;
55
import android.os.Bundle;
6-
import android.os.Handler;
76
import androidx.recyclerview.widget.GridLayoutManager;
87
import androidx.recyclerview.widget.LinearLayoutManager;
98
import androidx.recyclerview.widget.RecyclerView;
@@ -136,12 +135,12 @@ public void updateCategoryList(String query) {
136135
progressBar.setVisibility(GONE);
137136
queryList.clear();
138137
categoriesAdapter.clear();
139-
Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size()))
138+
compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size()))
140139
.subscribeOn(Schedulers.io())
141140
.observeOn(AndroidSchedulers.mainThread())
142141
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
143142
.doOnSubscribe(disposable -> saveQuery(query))
144-
.subscribe(this::handleSuccess, this::handleError);
143+
.subscribe(this::handleSuccess, this::handleError));
145144
}
146145

147146

@@ -152,11 +151,11 @@ public void addCategoriesToList(String query) {
152151
this.query = query;
153152
bottomProgressBar.setVisibility(View.VISIBLE);
154153
progressBar.setVisibility(GONE);
155-
Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size()))
154+
compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size()))
156155
.subscribeOn(Schedulers.io())
157156
.observeOn(AndroidSchedulers.mainThread())
158157
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
159-
.subscribe(this::handlePaginationSuccess, this::handleError);
158+
.subscribe(this::handlePaginationSuccess, this::handleError));
160159
}
161160

162161
/**

0 commit comments

Comments
 (0)