IlegalStateException when onItemClick after restart

Question

To reproduce, get the SSCCE Android Project on Github and :

Touch the hamburger to display navigation menu
Select Employees
Select an Employee
Touch the back button
Touch the Overview button
Select the application from the list
Touch the hamburger to display navigation menu
Select Employees
Select an Employee => IllegalStateException

java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState at android.support.v4.app.FragmentManagerImpl.checkStateLoss(FragmentManager.java:1538) at android.support.v4.app.FragmentManagerImpl.enqueueAction(FragmentManager.java:1556) at android.support.v4.app.BackStackRecord.commitInternal(BackStackRecord.java:696) at android.support.v4.app.BackStackRecord.commit(BackStackRecord.java:662) at example.com.replacefragments_onitemclick.fragments.FragmentChange.onFragmentChange(FragmentChange.java:88)

fragmentTransaction.commit(); // IllegalStateException:  FragmentChange.java:88

The reason for the exception is clear: With the replace statement, it is trying to replace a fragment that is attached to a now non-existent Activity instance.

Overriding onSaveInstanceState() as suggested here has no effect.

Numerous questions suggest using commitAllowingStateLoss(). It does not fix the problem and apparently is kind of a hack anyway.

Also, there are answers that say to keep a reference to the old Activity. This does not seem right.

How can I prevent this exception?

Answer 1

In FragmentChange you use a Singleton design pattern, In your first launch when you click on an employee the you set the FragmentManager through your FragmentActivity which already exists, when press back the activity is gone and when open the application again a new activity is created using savedInstance, But it is another object. But FragmentChange object is still there using the old Activity. you need to either not use the Singleton pattern or you update the FragmentManager each time you use it.

So in FragmentChange you either do

public static FragmentChange getInstance(FragmentManager fragmentManager) {
    instance = new FragmentChange(fragmentManager);
    return instance;
}

or

public static FragmentChange getInstance(FragmentManager fragmentManager) {
    if (instance == null) {
        instance = new FragmentChange(fragmentManager);
    }
    instance.mFragmentManager = fragmentManager;
    return instance;
}

Answer 2

Here are some suggestions to keep in the back of your mind as you work with FragmentTransactions in your applications.

1. Be careful when committing transactions inside Activity life-cycle methods. A large majority of applications will only ever commit transactions the very first time onCreate() is called and/or in response to user input, and will never face any problems as a result. However, as your transactions begin to venture out into the other Activity life-cycle methods, such as onActivityResult(), onStart(), and onResume(), things can get a little tricky. For example, you should not commit transactions inside the FragmentActivity#onResume() method, as there are some cases in which the method can be called before the activity’s state has been restored (see the documentation for more information). If your application requires committing a transaction in an Activity life-cycle method other than onCreate(), do it in either FragmentActivity#onResumeFragments() or Activity#onPostResume(). These two methods are guaranteed to be called after the Activity has been restored to its original state, and therefore avoid the possibility of state loss all together. (As an example of how this can be done, check out this answer).

2. Avoid performing transactions inside asynchronous callback methods. This includes commonly used methods such as AsyncTask#onPostExecute() and LoaderManager.LoaderCallbacks#onLoadFinished(). The problem with performing transactions in these methods is that they have no knowledge of the current state of the Activity life-cycle when they are called. For example, consider the following sequence of events:

   • An activity executes an AsyncTask.

   • The user presses the “Home” key, causing the activity’s onSaveInstanceState() and onStop() methods to be called.

   • The AsyncTask completes and onPostExecute() is called, unaware that the Activity has since been stopped.

   • A FragmentTransaction is committed inside the onPostExecute() method, causing an exception to be thrown.

   In general, the best way to avoid the exception in these cases is to simply avoid committing transactions in asynchronous callback methods all together. Google engineers seem to agree with this belief as well. According to this post on the Android Developers group, the Android team considers the major shifts in UI that can result from committing FragmentTransactions from within asynchronous callback methods to be bad for the user experience. If your application requires performing the transaction inside these callback methods and there is no easy way to guarantee that the callback won’t be invoked after onSaveInstanceState(), you may have to resort to using commitAllowingStateLoss() and dealing with the state loss that might occur. (See also these two SO posts for additional hints, here and here).

3. Use commitAllowingStateLoss() only as a last resort. The only difference between calling commit() and commitAllowingStateLoss() is that the latter will not throw an exception if state loss occurs. Usually you don’t want to use this method because it implies that there is a possibility that state loss could happen. The better solution, of course, is to write your application so that commit() is guaranteed to be called before the activity’s state has been saved, as this will result in a better user experience. Unless the possibility of state loss can’t be avoided, commitAllowingStateLoss() should not be used.

For more information on Activity state loss and its impact on FragmentTransaction check this link.

Hope this will help you. Happy coding!!!

Answer 3

The general problem is losing the context. You are right that keeping a reference to the old Activity is a bad way because the Android system should manage the Activity's lifecycle. But you kept the reference to FragmentManager in FragmentChange class when static FragmentChange instance is created (FragmentChange.java : 46).

So what really happens: you created static instance of FragmentChange. The FragmentChange instance keeps reference to FragmentManager that is linked with MainActivity instance . When you press back, the system calls the MainActivity instance lifecycle callback. It calls onStop() callback that also invokes dispatchStop() in FragmentManager instance where to mStateSaved variable is assigned to true. And after that the MainActivity instance is destroyed. But FragmentChange instance is kept because the application isn't destroyed, just the activity instance . When you return to the app, the new MainActivity instance will be created. But your FragmentChange still keeps the reference to the old FragmentManager instance that is linked to the dead MainActivity instance . So no one will call dispatchCreate(), or dispatchResume(), or any other method to restore the mStateSaved value to false, the old FragmentManager instance isn't linked to the app's life cycle. And when you select Employees once again, the old FragmentManager instance throws IllegalStateException because mStateSaved still has true value.

The general solution is not to create the static reference to Activity, Fragment, View, FragmentManager and so on. In general, to all classes which lifecycle is Android system business. The one of the possible solutions for your case is not to keep a reference to the FragmentManager but to send it as a parameter to onFragmentChange. I provided a code example in the pull request https://github.com/emnrd-ito/ReplaceFragment-OnItemClick/pull/1/files