Mosby 3.1.0 NPEs in Activity#onDestroy()

laalto commented 6 years ago

After upgrading app to Mosby 3.1.0 crash analytics have started displaying crashes like:

java.lang.RuntimeException: Unable to destroy activity {p/p.a}: java.lang.NullPointerException: Attempt to invoke interface method 'void com.hannesdorfmann.mosby3.mvp.MvpPresenter.detachView()' on a null object reference
    at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:4299)
    at android.app.ActivityThread.handleDestroyActivity(ActivityThread.java:4317)
    at android.app.ActivityThread.-wrap6(ActivityThread.java)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1569)
    at android.os.Handler.dispatchMessage(Handler.java:102)
    at android.os.Looper.loop(Looper.java:241)
    at android.app.ActivityThread.main(ActivityThread.java:6274)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
Caused by: java.lang.NullPointerException: Attempt to invoke interface method 'void com.hannesdorfmann.mosby3.mvp.MvpPresenter.detachView()' on a null object reference
    at com.hannesdorfmann.mosby3.mvp.delegate.ViewGroupMvpDelegateImpl.detachPresenterIfNotDoneYet(ViewGroupMvpDelegateImpl.java:301)
    at com.hannesdorfmann.mosby3.mvp.delegate.ViewGroupMvpDelegateImpl.onActivityDestroyed(ViewGroupMvpDelegateImpl.java:255)
    at android.app.Application.dispatchActivityDestroyed(Application.java:253)
    at android.app.Activity.onDestroy(Activity.java:1851)
    at android.support.v4.app.FragmentActivity.onDestroy(FragmentActivity.java:358)
    at android.support.v7.app.AppCompatActivity.onDestroy(AppCompatActivity.java:209)
    at p.a.onDestroy(a.java:)
    at android.app.Activity.performDestroy(Activity.java:6922)
    at android.app.Instrumentation.callActivityOnDestroy(Instrumentation.java:1154)
    at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:4286)
    ... 9 more

Do not have the specific repro steps available yet, but reading the source in ViewGroupMvpDelegateImpl:

  private void destroyPresenterIfnotDoneYet() {
    if (!presenterDestroeyed) {
      P presenter = delegateCallback.getPresenter();
      presenter.destroy();

To me this looks like the code is assuming too much about the fragment/activity lifecycle - the fragment lifecycle has not reached MvpFragment#onCreate() where the presenter is created but the the activity gets destroyed for some other reason, and presenterDestroeyed defaults to false.

For what it's worth, there's also a typo in presenterDestroeyed.

sockeqwe commented 6 years ago

Hey thanks for reporting. How do you use your ViewGroup, do you remove that one manually? How can I reproduce this bug? Did it work in 3.0.4?

laalto commented 6 years ago

So far only seen this is crash reports sent via automated tooling, no repro cases reported. I'll try to cook up one in near future.

Didn't receive such crash reports before Mosby 3.1.0 update and that lead me to believe it's related as there were lifecycle-related changes (such as MvpFragment createPresenter() getting invoked in onCreate() rather than onViewCreated() fragment lifecycle phase).

The app itself is large with tens of developers contributing. In most places the UI is based on MvpFragment with MvpNullObjectBasePresenter. But now that you mention it, I cannot exclude that there is somewhere e.g. MvpView-based approaches (with potential lifecycle-related bugs). Have to recheck when back at work and not busy with something else.

Also noting that I posted the wrong code snippet. It does have a typo, but the crash is in the next private method. Sorry about that.

laalto commented 6 years ago

Ok, finally got some time to debug this deeper. No original repro steps known but here's something that produces similar stacktraces:

The app has a bottom navigation bar that is-a MvpFrameLayout (and hence ViewGroupMvpDelegateImpl seen in stacktrace)
Each post-login activity in the app checks for a valid session in onResume()and if the session is stale, calls finish() to go back to login activity
ViewGroupMvpDelegateImpl starts listening for activity lifecycle changes once instantiated but only inits the presenter in onAttachedToWindow() which runs after activity onResume()

Here's a sample activity that demonstrates the above:

public class MosbyBug290Activity extends AppCompatActivity {

    static class BottomNaviPresenter extends MvpBasePresenter<MvpView> {
    }

    static class BottomNavi extends MvpFrameLayout<MvpView, BottomNaviPresenter> {

        public BottomNavi(Context ctx) {
            super(ctx);
        }

        @Override
        public BottomNaviPresenter createPresenter() {
            return new BottomNaviPresenter();
        }

        // override to extend visibility from protected to public
        @Override
        public Parcelable onSaveInstanceState() {
            return super.onSaveInstanceState();
        }
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        BottomNavi navi = new BottomNavi(this);
        setContentView(navi);

        // Do something that forces ViewGroupMvpDelegateImpl to be constructed
        // and register itself to listen for activity lifecycle changes
        navi.onSaveInstanceState();
    }

    @Override
    protected void onResume() {
        super.onResume();

        // Simulate a session timeout that throws the user back to login activity
        finish();

        // onAttachedToWindow() where the presenter is initialized only runs after activity onResume()
        // and hence receiving a crash here
    }
}

sockeqwe commented 6 years ago

I see, thanks for the detailed report

martinflorek commented 6 years ago

Anything new regarding this issue? I think that I just hit the same bug in v 3.1.0, but with MviPresenter and the Conductor library:

java.lang.NullPointerException: Attempt to invoke interface method 'void com.hannesdorfmann.mosby3.mvi.MviPresenter.destroy()' on a null object reference
    at com.hannesdorfmann.mosby3.MviConductorLifecycleListener.postDestroy(MviConductorLifecycleListener.java:133)

My app has a complex routing with several nested routers in Conductor and this crash happens when the app is resumed after a long time in background into state with backstack bigger then one. Then after tapping the back it does not pop the backstack but crashes.

It can be reproduced with the "Don't keep activities" enabled in the debug options in the phone's settings. Then navigating to some screen where the backstack grows and minimizing the app (i.e. press home) will crash the app, sometimes :/

drampelt commented 6 years ago

A similar error happens when you call finish for an activity in onCreate, it tries to delete a null presenter in onDestroy and crashes.

sockeqwe commented 6 years ago

I will work on this kind of issues this weekend

Daniel Rampelt notifications@github.com schrieb am Do., 29. März 2018, 20:55:

A similar error happens when you call finish for an activity in onCreate, it tries to delete a null presenter in onDestroy and crashes.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/290#issuecomment-377337195, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrhB-LFSwnhhJPZdgshgXm7IZqH9gks5tjS4pgaJpZM4Q9cib .

Vivecstel commented 6 years ago

when the new update will be released? I am also getting this exception on crashlytics on some users:

Caused by java.lang.NullPointerException at com.hannesdorfmann.mosby3.mvp.delegate.ViewGroupMvpDelegateImpl.detachPresenterIfNotDoneYet(ViewGroupMvpDelegateImpl.java:301) at com.hannesdorfmann.mosby3.mvp.delegate.ViewGroupMvpDelegateImpl.onActivityDestroyed(ViewGroupMvpDelegateImpl.java:255) at android.app.Application.dispatchActivityDestroyed(Application.java:265) at android.app.Activity.onDestroy(Activity.java:1541) at android.support.v4.app.FragmentActivity.onDestroy(FragmentActivity.java:358) at android.support.v7.app.AppCompatActivity.onDestroy(AppCompatActivity.java:209) ...

diogojg commented 5 years ago

I'm also detecting this crash from time to time and on Crashlytics. @sockeqwe Are going to release a new update anytime soon? Thanks!

sockeqwe commented 5 years ago

Working on it, along migrating to android x

keith30xi commented 5 years ago

Is there a chance to get this fixed without tying it to the conversion to androidx? We are seeing crashes as well.

sockeqwe commented 5 years ago

Use latest snapshot 3.1.1-SNAPSHOT (see README)

Keith Naas notifications@github.com schrieb am Fr., 16. Nov. 2018, 16:41:

Is there a chance to get this fixed without tying it to the conversion to androidx? We are seeing crashes as well.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/290#issuecomment-439433162, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnruqdbBGcIrU_o0r_zNj1bVOaW75Uks5uvtyygaJpZM4Q9cib .

Burtsev-Ivan commented 5 years ago

Version 3.1.1-SNAPSHOT still fails

vladimirfx commented 5 years ago

Reproducable on 3.1.1 release. ViewGroupMviDelegateImpl I've call finish() in onResume and onActivityResult

sockeqwe commented 5 years ago

Thanks for reporting. Will check this later this week. Just to be clear: it causes to crash in both cases:

calling finish() in onResume()
calling finish() in onActivityResult()

vladimirfx commented 5 years ago

I don't know actually. Stacktrace do not include either:

Caused by java.lang.NullPointerExceptionAttempt to invoke interface method 'void com.hannesdorfmann.mosby3.mvp.MvpPresenter.detachView()' on a null object reference Raw Text
--
  | com.hannesdorfmann.mosby3.ViewGroupMviDelegateImpl.detachPresenterIfNotDoneYet (ViewGroupMviDelegateImpl.java:275)
  | com.hannesdorfmann.mosby3.ViewGroupMviDelegateImpl.onActivityDestroyed (ViewGroupMviDelegateImpl.java:251)
  | android.app.Application.dispatchActivityDestroyed (Application.java:253)
  | android.app.Activity.onDestroy (Activity.java:1995)
  | android.support.v4.app.FragmentActivity.onDestroy (FragmentActivity.java:413)
  | android.support.v7.app.AppCompatActivity.onDestroy (AppCompatActivity.java:210)
  | android.app.Activity.performDestroy (Activity.java:7297)
  | android.app.Instrumentation.callActivityOnDestroy (Instrumentation.java:1250)
  | android.app.ActivityThread.performDestroyActivity (ActivityThread.java:4437)
  | android.app.ActivityThread.handleDestroyActivity (ActivityThread.java:4468)
  | android.app.ActivityThread.-wrap5 (Unknown Source)
  | android.app.ActivityThread$H.handleMessage (ActivityThread.java:1678)
  | android.os.Handler.dispatchMessage (Handler.java:106)
  | android.os.Looper.loop (Looper.java:171)
  | android.app.ActivityThread.main (ActivityThread.java:6651)
  | java.lang.reflect.Method.invoke (Method.java)
  | com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:547)
  | com.android.internal.os.ZygoteInit.main (ZygoteInit.java:824)

sockeqwe / mosby

Mosby 3.1.0 NPEs in Activity#onDestroy() #290