Skip to content

Fix memory leaks by clearing view bindings and references in fragments #6347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ public void onBackStackChanged() {

}

@Override
public void onDestroyView() {
super.onDestroyView();
binding = null;
}

@Override
public void onDestroy() {
super.onDestroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ class BookmarkLocationsFragment : DaggerFragment() {
}
}

override fun onDestroyView() {
super.onDestroyView()
binding = null
}

override fun onDestroy() {
super.onDestroy()
// Make sure to null out the binding to avoid memory leaks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ public void onStop() {
controller.stop();
}

@Override
public void onDestroyView() {
super.onDestroyView();
binding = null;
}

@Override
public void onDestroy() {
super.onDestroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,14 +685,23 @@ class ContributionsFragment : CommonsDaggerSupportFragment(), FragmentManager.On
override fun onDestroy() {
try {
compositeDisposable.clear()
// Remove child fragment safely
contributionsListFragment?.let {
childFragmentManager.beginTransaction()
.remove(it)
.commitAllowingStateLoss()
}
childFragmentManager.removeOnBackStackChangedListener(this)
locationManager!!.unregisterLocationManager()
locationManager!!.removeLocationListener(this)
super.onDestroy()
locationManager?.unregisterLocationManager()
locationManager?.removeLocationListener(this)
// Nullify locationManager to prevent leaks
locationManager = null
} catch (exception: IllegalArgumentException) {
Timber.e(exception)
} catch (exception: IllegalStateException) {
Timber.e(exception)
} finally {
super.onDestroy()
}
}

Expand Down Expand Up @@ -757,7 +766,9 @@ class ContributionsFragment : CommonsDaggerSupportFragment(), FragmentManager.On

override fun onDestroyView() {
super.onDestroyView()
presenter!!.onDetachView()
presenter?.onDetachView()
binding = null
contributionsListFragment = null
}

override fun notifyDataSetChanged() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,11 @@ after opening the app.
quizChecker!!.cleanup()
locationManager!!.unregisterLocationManager()
// Remove ourself from hashmap to prevent memory leaks
try {
locationManager?.unregisterLocationManager()
} catch (e: Exception) {
Timber.e(e, "Error while cleaning up locationManager")
}
locationManager = null
super.onDestroy()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ class MediaDetailFragment : CommonsDaggerSupportFragment(), CategoryEditHelper.C
}

compositeDisposable.clear()

_binding = null
super.onDestroyView()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ class ProfileActivity : BaseActivity() {
public override fun onDestroy() {
super.onDestroy()
compositeDisposable.clear()
// Nullify fragment references to avoid memory leaks
if (::achievementsFragment.isInitialized) achievementsFragment.setHasOptionsMenu(false)
if (::leaderboardFragment.isInitialized) leaderboardFragment.setHasOptionsMenu(false)
contributionsFragment = null
}

override fun onCreateOptionsMenu(menu: Menu): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,8 @@ class UploadActivity : BaseActivity(), UploadContract.View, UploadBaseFragment.C
uploadCategoriesFragment!!.callback = null
}
onBackPressedCallback.remove()
locationManager?.unregisterLocationManager()
UploadMediaPresenter.presenterCallback = null // Clearing reference
}

/**
Expand Down
10 changes: 0 additions & 10 deletions app/src/main/res/values-x-invalidLanguageCode/error.xml

This file was deleted.

4 changes: 2 additions & 2 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,8 @@
<string name="statistics_thanks">Thanks Received</string>
<string name="statistics_featured">Featured Images</string>
<string name="statistics_wikidata_edits">Images via \"Nearby Places\"</string>
<string name="level">Level %d</string>
<string name="profileLevel">%s (Level %s)</string>
<string name="level" formatted="false">Level %d</string>
<string name="profileLevel" formatted="false">%s (Level %s)</string>
<string name="images_uploaded">Images Uploaded</string>
<string name="image_reverts">Images Not Reverted</string>
<string name="images_used_by_wiki">Images Used</string>
Expand Down