Closeable objects are not closing if they are added after 'onClear' [280294730]

Fixed

Bug

Status Update

No update yet.

Description

tr...@gmail.com

created issue #1

May 2, 2023 12:27PM

Component used: androidx.lifecycle:lifecycle-viewmodel

Version used: 2.6.1

Devices/Android versions reproduced on: n/a

Current implementation of addCloseable doesn't have a check of mCleared, unlike of putTagIfAbsent. It may lead to memory leaks, because user can not add check of mCleared and close Closeable by its own. For example, if we start background task (thread, coroutine, rx-java chain), it is possible for this task to be active after onClear invocation (even this time is small) and add Closeable object after onClear. In this case, Closeable.close() will not be invoked.

Could be this check added to addCloseable implementation?

Comments

il...@google.com <il...@google.com> #2May 8, 2023 06:29PM

Assigned to sa...@google.com.

Jeremy, is this still an issue? I think the problem was that you had two transitions targeting the same View for the same action (e.g. two Slide() transitions).

sa...@google.com <sa...@google.com> Aug 23, 2023 04:08PM

Accepted by sa...@google.com.

ap...@google.com <ap...@google.com> #3Aug 24, 2023 06:45PM

Marked as fixed.

I have a similar issue with plain AnimatorSet:

set.start()
set.pause()
set.setCurrentPlayTime(100)
set.setCurrentPlayTime(0)
set.setCurrentPlayTime(100)
set.resume()

doesn't play animation in resume().

il...@google.com <il...@google.com> #4Aug 24, 2023 11:40PM

Should clarify that if I filter out setCurrentPlayTime(0) (or replace it with setCurrentPlayTime(1)) it works well.

Also even with setCurrentPlayTime(0), onAnimationEnd is notified with correct delay (as if the animation has played).

pr...@google.com <pr...@google.com> #5Sep 6, 2023 07:56PM

@ #4:

I think that is intended for Animator. If you set the currentPlayTime to 0 or the total duration the animator completes. We do some special checks in fragments to handle it.

ph...@gmail.com <ph...@gmail.com> #6Mar 6, 2024 04:05PM

Did some investigation on the Fragment side and it seems like the merged transition is targeting correctly.

Exiting Transition: Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
>>>>> ExitingViews <<<<<
View: android.widget.LinearLayout{f9add3d}
Entering Transition: Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
>>>>> EnteringViews <<<<<
View: android.widget.LinearLayout{b7f24bc}
Final merged transition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})
merged transition passed to controlDelayedTransition: TransitionSet@7bc1c45: 
      TransitionSet@e133f9a: 
        Slide@aa9288e: tgts(android.widget.LinearLayout{f9add3d})
      Slide@35b8af: tgts(android.widget.LinearLayout{b7f24bc})

Still digging.

Issue 280294730