AutoDispose icon indicating copy to clipboard operation
AutoDispose copied to clipboard

Leak - DetachEventCompletable isn't removed when View is removed from ViewTree

Open ultraon opened this issue 3 years ago • 1 comments

https://github.com/uber/AutoDispose/blob/7d86ab1d191e2e9c78a2ccf05ea514ad0ca0dc71/android/autodispose-android/src/main/java/com/uber/autodispose/android/DetachEventCompletable.java?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L79

Hello, Thank you very much for that great library.

I think I found a leak when DetachEventCompletable.Listener isn't removed from listeners inside a View. And this view was multiple times added and removed from ViewTree.

Description:

  1. when this button is clicked, observe the number of listeners in android.view.View.ListenerInfo#mOnAttachStateChangeListeners using debug break-points
  2. Click the button "Add" and then "Remove" a few times
  3. Then observe the number of listeners in android.view.View.ListenerInfo#mOnAttachStateChangeListeners again

Result: the com.uber.autodispose.android.DetachEventCompletable listeners are not removed using android.view.View.removeOnAttachStateChangeListener that should be done in com.uber.autodispose.android.DetachEventCompletable.Listener.onViewDetachedFromWindow.

Probably the fix would be simple:

@Override
public void onViewDetachedFromWindow(View v) {
  if (!isDisposed()) {
    observer.onComplete();
    v.removeOnAttachStateChangeListener(this) // <---------- this is a fix.
  }
}

Sample project: TestUberAutodisposeApp.zip

ultraon avatar Feb 04 '22 21:02 ultraon

Nice catch! Want to send a PR?

ZacSweers avatar Feb 13 '22 18:02 ZacSweers