fix: prevent memory leaks in CoordinatorLayout and ViewPager2 - #16
Open
Lemkinator wants to merge 3 commits into
Open
fix: prevent memory leaks in CoordinatorLayout and ViewPager2#16Lemkinator wants to merge 3 commits into
Lemkinator wants to merge 3 commits into
Conversation
… prevent memory leak Samsung's SESL fork stores mLastNestedScrollingChild as a strong View reference and never clears it when the scrolling child is removed from the window. This prevents detached fragment view hierarchies from being GC'd after navigation. Use WeakReference<View> instead, matching AOSP's original behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ent in onDetachedFromWindow onStopNestedScroll already assigns mLastNestedScrollingChild; the preceding assignment was redundant and caused an unnecessary WeakReference allocation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FragmentMaxLifecycleEnforcer registers mLifecycleObserver on the host fragment lifecycle. The observer captures the enforcer, which holds mViewPager, keeping the ViewPager2 and all child fragment views alive after the host fragment's view is destroyed. Fix: add an OnAttachStateChangeListener to the ViewPager2 that nulls mViewPager on window-detach and restores it on reattach. Add a null guard in updateFragmentMaxLifecycle() accordingly. unregister() also removes the attach state listener. The lifecycle observer is intentionally kept registered so it can still catch up on lifecycle state changes if the ViewPager2 reattaches.
Lemkinator
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CoordinatorLayout
mLastNestedScrollingChildchanged from strongViewref toWeakReference<View>— prevents detached fragment views from being retained after back-stack navigationonDetachedFromWindow(the subsequentonStopNestedScrollcall already performs the same assignment)new WeakReference<>(view)ornull; the one read site indispatchGenericMotionEventresolves via.get()into a local before useViewPager2 / FragmentStateAdapter
FragmentStateAdapter(Fragment)registersmLifecycleObserveron the host fragment lifecycle (not view lifecycle). The observer capturesFragmentMaxLifecycleEnforcer→mViewPager→ child fragment views, retaining ~7 MB after the host fragment's view is destroyedFragmentMaxLifecycleEnforcernow adds anOnAttachStateChangeListenerto theViewPager2that nullsmViewPageron window-detach and restores it on reattach, breaking the retain chain without removing the lifecycle observerupdateFragmentMaxLifecycle()guarded with null check;unregister()also removes the attach state listenerTest plan
CoordinatorLayoutand verify LeakCanary reports nomLastNestedScrollingChild-related leaksViewPager2withFragmentStateAdapterand verify LeakCanary reports no VP2 / child fragment view leaks