Skip to content
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

[BottomSheet] Fix SettleRunnable leak #1670

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pyricau
Copy link

@pyricau pyricau commented Aug 31, 2020

Ensures that we don't post the SettleRunnable if the view isn't attached, because otherwise if there is no alive activity that SettleRunnable can stay in the ViewRootImpl RunQueue until a new activity starts.

Fixes #1417.

Ensures that we don't post the `SettleRunnable` if the view isn't attached, because otherwise if there is no alive activity that `SettleRunnable` can stay in the ViewRootImpl `RunQueue` until a new activity starts.

Fixes material-components#1417.
@@ -1324,7 +1324,7 @@ void startSettlingAnimation(View child, int state, int top, boolean settleFromVi
settleRunnable = new SettleRunnable(child, state);
}
// If the SettleRunnable has not been posted, post it with the correct state.
if (settleRunnable.isPosted == false) {
if (!settleRunnable.isPosted) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated but the IDE warning was in my face so I couldn't resist.

Copy link
Contributor

@drchen drchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, can you explain why you think this will happen in the first place? SettleRunnable can only be posted when onViewReleased() is called, at which moment it doesn't seem likely the view is detached....

@pyricau
Copy link
Author

pyricau commented Sep 14, 2021

@drchen sorry just catching up with GitHub notifications.

That was a few months back so I don't recall all the details, but I did write this in the issue:

BottomSheetBehavior.startSettlingAnimation() calls ViewCompat.postOnAnimation(child, settleRunnable);.

To be clear, I haven't reproduced the bug directly, this is all based on leak reports which show this happening

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants