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

7.1内存泄漏 #21

Open
RebornWolfman opened this issue Aug 19, 2021 · 6 comments
Open

7.1内存泄漏 #21

RebornWolfman opened this issue Aug 19, 2021 · 6 comments
Labels

Comments

@RebornWolfman
Copy link
Contributor

RebornWolfman commented Aug 19, 2021

当使用 removeObservers(@nonnull final LifecycleOwner owner) 的时候,回调过来已经是ObserverWrapper,又封装了一层,导致没有删除掉。应该是加一个判断,判断回调当前类是否已经是ObserverWrapper的类型,不是才需要包装删除,那时候没有测试想到。
@OverRide
public void removeObserver(@nonnull @NotNull Observer<? super T> observer) {
if (observer.getClass().isAssignableFrom(ObserverWrapper.class)) {
super.removeObserver(observer);
} else {
super.removeObserver(createObserverWrapper(observer, START_VERSION, false));
}
}

@KunMinX
Copy link
Owner

KunMinX commented Aug 19, 2021

@RebornWolfman

感谢反馈,我在 7.1 中是采取了以下方式区分的 Observe 和 ObserveForever 的情况从而规避 7.0 的内存泄漏,

你是指最新 7.1 版本的下述代码仍然存在内存泄漏是吗

@Override
  public void removeObserver(@NonNull Observer<? super T> observer) {
    if (TextUtils.isEmpty(observer.toString())) {
      super.removeObserver(observer);
    } else {
      super.removeObserver(createObserverWrapper(observer, -1));
    }
  }

@KunMinX
Copy link
Owner

KunMinX commented Aug 19, 2021

@RebornWolfman

明白了,原来是重载方法 removeObservers(LifecycleOwner owner)

我去做个测试,感谢~

@KunMinX KunMinX added 精华 and removed 精华 labels Aug 19, 2021
@KunMinX
Copy link
Owner

KunMinX commented Aug 19, 2021

@RebornWolfman

刚刚我为本项目 sample 中的 “多观察者测试” 页面添加了 “removeObservers(owner)” 的测试,基于 7.1 的代码,无法复现 “无法移除” 的问题,

换上 isAssignableFrom 来判断,在 “forever观察者页面” 旋屏重建多次时有一定概率发生内存泄漏(猜测会不会是 isAssignableFrom 性能所致)

具体可拉取最新的 sample 试一试

@RebornWolfman
Copy link
Contributor Author

RebornWolfman commented Aug 19, 2021


    @Override
    public void removeObserver(@NonNull Observer<? super T> observer) {
        if (observer.getClass().isAssignableFrom(ObserverWrapper.class)) {
            super.removeObserver(observer);
        } else {
            super.removeObserver(createObserverWrapper(observer, START_VERSION));
        }
//    if (TextUtils.isEmpty(observer.toString())) {
//      super.removeObserver(observer);
//    } else {
//      super.removeObserver(createObserverWrapper(observer, START_VERSION));
//    }
    }
   我原来没有测试过,我只是看,你那个是用多了一个字符串去过滤Observer,效果是一样的;我使用isAssignableFrom ,
测试了 “forever观察者页面”也试过多次旋转屏幕 ,大概测试了十几分钟,没有发现删除不了的。

@KunMinX
Copy link
Owner

KunMinX commented Aug 20, 2021

@RebornWolfman

仅仅是调试的话,两种方式结果都一样,

区别就在于,快速旋转屏幕、让页面重建的次数多了,isAssignableFrom 这种容易收到 leakCanary 的内存泄漏警告,

要说逻辑的话,isAssignableFrom 这种确实更简便易懂,不用像 7.1 这样,额外给 wrapper 加个 isForever 并且重写 toString 来判断。

@KunMinX
Copy link
Owner

KunMinX commented Aug 20, 2021

为排除潜在的干扰因素,刚刚为 sample 的 ForeverObserve 页面添加了 onDestroy 自动移除 ForeverObserver 的脚本,

经过反复测试,v7.1 的方案和 isAssignableFrom 都无内存泄漏问题,

鉴于 isAssignableFrom 的逻辑更简便易懂,我们基于该方案升级框架到 v7.2 版本,

再次感谢 @RebornWolfman 的分享

@KunMinX KunMinX added 精华 and removed 讨论中 labels Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants