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

Mosby 3.1.0 NPEs in Activity#onDestroy() #290

Open
laalto opened this issue Dec 11, 2017 · 16 comments
Open

Mosby 3.1.0 NPEs in Activity#onDestroy() #290

laalto opened this issue Dec 11, 2017 · 16 comments

Comments

@laalto
Copy link
Contributor

laalto commented Dec 11, 2017

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
Copy link
Owner

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
Copy link
Contributor Author

laalto commented Dec 11, 2017

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
Copy link
Contributor Author

laalto commented Dec 19, 2017

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
    }
}

laalto pushed a commit to laalto/mosby that referenced this issue Dec 19, 2017
@sockeqwe
Copy link
Owner

I see, thanks for the detailed report

@martinflorek
Copy link

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
Copy link

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
Copy link
Owner

sockeqwe commented Mar 29, 2018 via email

sockeqwe added a commit that referenced this issue Apr 9, 2018
sockeqwe added a commit that referenced this issue Apr 9, 2018
@Vivecstel
Copy link

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)
...

sockeqwe pushed a commit that referenced this issue Apr 24, 2018
@diogojg
Copy link

diogojg commented Sep 24, 2018

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
Copy link
Owner

Working on it, along migrating to android x

@keith30xi
Copy link

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

@sockeqwe
Copy link
Owner

sockeqwe commented Nov 16, 2018 via email

@Burtsev-Ivan
Copy link

Version 3.1.1-SNAPSHOT still fails

@vladimirfx
Copy link

vladimirfx commented Mar 6, 2019

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

@sockeqwe
Copy link
Owner

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

  1. calling finish() in onResume()
  2. calling finish() in onActivityResult()

@vladimirfx
Copy link

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)


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants