DroidKaigi / conference-app-2018

The Official Conference App for DroidKaigi 2018 Tokyo
Apache License 2.0
1.35k stars 332 forks source link

Memory leak by tab logic #590

Closed takahirom closed 6 years ago

takahirom commented 6 years ago

Overview (Required)

image

ogapants commented 6 years ago

https://twitter.com/new_runnable/status/960062516251340800 I'll check it!

ogapants commented 6 years ago

@takahirom Sorry, Could you tell me the procedure for reproducing?

takahirom commented 6 years ago

@ogapants I don't have reproducing method. 😭 But I think Kotlin object is the singleton. So it is leaking. Can you stop cache fragment? like this https://github.com/DroidKaigi/conference-app-2018/pull/493/files#diff-8fa8830b5e88e5e809ec4e5ea4be2e6cR195

ogapants commented 6 years ago

@takahirom ご迷惑おかけしてすみません! ちょっと2点確認させてください。

1. ちょっと最新のmasterのdebug版でBottomNaviの検索周りをいじってみてもそのleakcanaryが再現しないのですが、↓こちらでのFragmentの取得もそのような取得方法を取ればいいですかね? https://github.com/ogapants/conference-app-2018/blob/3da6872e19921cdc20a3947bdacee3cc3f1a2cbc/app/src/main/java/io/github/droidkaigi/confsched2018/presentation/search/SearchFragment.kt#L238-L238 手元で事前に再現させて見たいので、もし操作する上で決まり手のようなものがあれば教えていただきたいです。

  1. また、このPRでFragmentの取得方法を変更したのは理由があります。 今までの方法だと、OnTabReselectedListenergetItem()が呼ばれると新たにnewInstanceされたFragmentが返り、scrollToTop()内のbindingが初期化されてない状態なのでkotlin.UninitializedPropertyAccessException: lateinit property binding has not been initializedが起きます。 今は取得方法が元に戻ったので、現在のタブを再選択するとこの問題でクラッシュしてしまいます 😭 ↓を参考に、instantiateItemを使って現在のFragmentを取得する方法を取ってみます! https://qiita.com/chooblarin/items/88b4accac0cbb6944d4b

takahirom commented 6 years ago

@ogapants

  1. ちょっといじっているのですがなかなか、、 🤔
  2. masterだと入っています 🙇
ogapants commented 6 years ago

@takahirom なるほど!すみません!1だけやります!!!

takahirom commented 6 years ago

検索タブ開いて終了を繰り返して極稀にという感じですね。。 image

ogapants commented 6 years ago

手元でもなにかをきっかけに稀に再現することを確認しました、、、↑の対応取ってみたところ発生しなくなった気がするので、とりあえずPR投げてみますね 🙏