square / mortar

A simple library that makes it easy to pair thin views with dedicated controllers, isolated from most of the vagaries of the Activity life cycle.
Apache License 2.0
2.16k stars 155 forks source link

Scope was destroyed before onSaveInstanceState #87

Open niqo01 opened 10 years ago

niqo01 commented 10 years ago

Do you have any idea on what could cause the activity scope to be destroyed before the same activity performSaveInstanceState is called?

Crash:

java.lang.IllegalStateException: Scope com.couchsurfing.mobile.ui.setup.SetupActivityBlueprint was destroyed
       at mortar.RealScope.assertNotDead(RealScope.java:150)
       at mortar.RealActivityScope.onSaveInstanceState(RealActivityScope.java:93)
       at com.couchsurfing.mobile.ui.base.BaseActivity.onSaveInstanceState(BaseActivity.java:100)
       at android.app.Activity.performSaveInstanceState(Activity.java:1152)
       at android.app.Instrumentation.callActivityOnSaveInstanceState(Instrumentation.java:1223)
       at android.app.ActivityThread.performStopActivityInner(ActivityThread.java:3156)
       at android.app.ActivityThread.handleStopActivity(ActivityThread.java:3215)
       at android.app.ActivityThread.access$1000(ActivityThread.java:135)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1424)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loop(Looper.java:137)
       at android.app.ActivityThread.main(ActivityThread.java:4998)
       at java.lang.reflect.Method.invokeNative(Method.java)
       at java.lang.reflect.Method.invoke(Method.java:515)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:777)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:593)
       at dalvik.system.NativeStart.main(NativeStart.java)
victorfu commented 10 years ago

Do you have similar sequence of operation: orientation changed --> back pressed (leave the app) --> start the app --> orientation changed?

rjrjr commented 10 years ago

This is definitely the trickiest part of getting Mortar to play nice. When, where and how are you calling destroy() on your activity scope?

One thing we've noticed is that there is some kind of race in Android's launcher where a new task will be started slightly overlapping an outgoing one (IIRC you can see this by really whaling on your app icon and the back button, or maybe it's the home button). We guard against this by including the Activity's task id in its Blueprint name. Maybe we need to build that into Mortar#requireActivityScope.

niqo01 commented 10 years ago

@victorfu nope

@rjrjr , We call destroy() on the activity scope in our Activity onDestroy method as follow:

    if (isFinishing() && activityScope != null) {
      activityScope.destroy();
      activityScope = null;
    }

I ll try adding the task id to activity scope. Apparently one of my beta user can reproduce this one almost consistently. I'll let you know more soon.

niqo01 commented 10 years ago

@rjrjr including the Activity's task id in its Blueprint name solve the issue. Maybe including this in the sample would be nice for others.

victorfu commented 10 years ago

@rjrjr your suggestion also solved my problem : )

rjrjr commented 10 years ago

The task id approach isn't perfect, we're still seeing the occasional problem where Android surprises us with overlapping activities that we weren't expecting. But I think @pyricau had a better idea yesterday — use onRetainNonConfigurationInstance, and a UUID in your activity scope name. We're trying it out now, and if it proves itself out I'll update the sample to demonstrate.

Something like:

  boolean configurationChangeIncoming;
  String scopeName;

  @Override protected final void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    activityScope = Mortar.requireActivityScope(getParentScope(), 
        new ActivityBlueprint(getScopeName()));
    activityScope.onCreate(savedInstanceState);
  }

  @Override public Object onRetainNonConfigurationInstance() {
    configurationChangeIncoming = true;
    return activityScope.getName();
  }

  @Override protected void onDestroy() {
    if (!configurationChangeIncoming) activityScope.destroy();
  }

  private String getScopeName() {
    if (scopeName == null) scopeName = (String) getLastNonConfigurationInstance();
    if (scopeName == null) {
      scopeName = getClass().getName() + "-" + UUID.randomUUID().toString();
    }

    return scopeName;
  }
rjrjr commented 9 years ago

Probably can avoid this mess by using singleTask or singleInstance launch mode. Should confirm that, or if it's not true bring this code to the sample.

lklots commented 8 years ago

FWIW singleTask and singleInstance cannot be relied on to solve this problem.

Lakedaemon commented 8 years ago

Including taskId in the blueprint (getScopeName()) fixed my espresso tests (they create and tear down activities/mortar scopes real fast and I ended up having dead MortarScopes)