sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

#290: Added null checks to prevent NPEs with ViewGroupDelegate cleanup #297

Closed laalto closed 6 years ago

laalto commented 6 years ago

and fixed some typos

sockeqwe commented 6 years ago

This looks good. Im just wondering if there is a better way to deal with that... Let me think about it this evening... Thanks!

laalto commented 6 years ago

Any news with this? Would want to get rid of the crashes but would not want to deploy a forked version of Mosby.

sockeqwe commented 6 years ago

Sorry, i was too busy lately and have forgotten about this. I have dedicated some time this weekend. This should be fixed by Monday. I appologize for any inconveniences this may have caused.

Lauri Aalto notifications@github.com schrieb am Mi., 17. Jan. 2018, 09:39:

Any news with this? Would want to get rid of the crashes but would not want to deploy a forked version of Mosby.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/pull/297#issuecomment-358234568, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrlbG4-6uqH-cTKI5ER4d7B3dt_Hoks5tLbHLgaJpZM4RHE38 .

sockeqwe commented 6 years ago

Hm, I tired to reproduce the actual issue to ensure this fixes the described issue.

So basically you start an activity and in Activity.onCreate() you call Activity.finish(). In the layout of the Activity there is some custom ViewGroup that is powered by Mosby such as a MvpFrameLayout, right?

laalto commented 6 years ago

In the real application almost like that: Code in Activity#onResume() checks for session validity and calls Activity#finishAffinity() to throw the user back to login screen in case the session is no longer valid. And yes, there's a bottom navigation bar in the top-level activity that is powered by MvpFrameLayout.

I have actually been doing some testing with this by deploying this fix in our internal QA builds and monitoring how it's doing. Before doing that there used to be a background radiation of 1..3 crashes a day from our internal QA, without QA people being able to come up with repro steps, only suspicions that "something times out in the background". There have been no more crashes after deploying this change.

sockeqwe commented 6 years ago

Ah I see, let me try to reproduce this with Activity#onResume() and Activity#finishAffinity()

sockeqwe commented 6 years ago

Thanks!