IllegalArgumentException when using Otto with a retained Fragment

Question

I am using Otto 1.3.3 and when I resume my application sometimes I get an IllegalArgumentException with the following stacktrace:

Caused by: java.lang.IllegalArgumentException: Producer method for type class 
com.couchsurfing.mobile.ui.setup
        .SessionProviderFragment$SessionConnectionStateChangeEvent found on 
        type class com.couchsurfing.mobile.ui.setup.SessionProviderFragment, 
        but already registered by type class 
        com.couchsurfing.mobile.ui.setup.SessionProviderFragment.
    at com.squareup.otto.Bus.register(Bus.java:194)
    at com.couchsurfing.mobile.ui.BaseRetainedFragment
       .onCreate(BaseRetainedFragment.java:20)

The SessionProviderFragment has its instance retained, please find below the extended class:

public abstract class BaseRetainedFragment extends SherlockFragment {

    @Inject
    Bus bus;

    @Override
    public void onCreate(final Bundle state) {
        super.onCreate(state);
        ((CouchsurfingApplication) getActivity().getApplication()).inject(this);
        setRetainInstance(true);
        bus.register(this);
    }

    @Override
    public void onDestroy() {
        super.onDestroy();
        bus.unregister(this);
        bus = null;
    }
}

I tried both using bus.register(this) in onAttach() or onCreate(), that didn't change the issue.

Shanan

Just a thought - perhaps you are leaking the activity that hosts the fragment, such that in some cases (e.g. device rotation) you are creating a second instance of that activity, with a second instance of the fragment. – Ejection

Answer 1

I am using one "Retained Fragment" per activity to save the state of an HTTP session request. My issue was that I didn't instantiate my "Retained Fragment" the proper way.

Before I had in onCreate():

if (savedInstanceState == null) {
    sessionProviderFragment = new SessionProviderFragment();
    getSupportFragmentManager().beginTransaction().add(sessionProviderFragment,
        SessionProviderFragment.TAG).commit();
}

Apparently the code above could create several SessionProviderFragment when quitting the activity is reopening it later. It seams that the correct way is :

sessionProviderFragment = (SessionProviderFragment) getSupportFragmentManager()
    .findFragmentByTag(SessionProviderFragment.TAG);

// If not retained (or first time running), we need to create it.
if (sessionProviderFragment == null) {
    sessionProviderFragment = new SessionProviderFragment();
    getSupportFragmentManager().beginTransaction().add(sessionProviderFragment,
            SessionProviderFragment.TAG).commit();
}
if (savedInstanceState == null) {
    initUiFragment();
}

I also moved the bus register/unregister in onResume/onPause in my BaseFragment to be sure that I will always have one SessionProviderFragment registered on the bus at a time.

Shanan

Answer 2

The proper place to register on the bus is in onResume() and the proper place to unregister is in onPause() like so:

public abstract class BaseRetainedFragment extends RoboSherlockFragment {
    @Inject private Bus bus;

    @Override
    public void onCreate(final Bundle state) {
        super.onCreate(state);
        setRetainInstance(true);
    }

    @Override
    public void onResume() {
        super.onResume();
        bus.register(this);
    }

    @Override
    public void onPause() {
        super.onDestroy();
        bus.unregister(this);
    }
}

Note that onDestroy() is not guaranteed to be called.

You might be about to comment on this and say, hey Chris, if I register in onResume() and and events are fired before I hit this method I won't receive the events! You would be right, but this means you aren't using Producers like you should be.

Also note, if you use roboguice-sherlock you don't have to inject yourself. You also don't need to null the Bus when the Fragment goes out of scope the garbage collector will clean it up for you.

Polyvalent

Regarding the comment about onDestroy(), Are you saying that otto may leak activities if we call unregister in onDestroy? When I read the docs for onDestroy(), I understand them to mean that onDestory should not be used for saving data, as the process may be spontaneously killed off/restarted (as seen the the activity lifecycle diagrams). In this case the activity (and otto) should all be released. – Marrero

Apart from special purposes, don't register/unregister OttoBus during respective onResume -> onPause with fragment. I just found a loophole. If a dialog is shown over the fragment that supposedly receives bus events, the onPause() on the fragment will get called and unexpectedly unregister the OttoBus. – Ungraceful

Answer 3

I've used Otto and EventBus mostly to pass updates from background services to Activities and Fragments. I don't know your exact use case, but the most common use for me was to update the UI (e.g. ProgressBar, status message, etc).

Having said that, what I've found as most efficient, is to register the bus in the onViewCreated() method of the fragment and unregister it in the onDestroyView() method. Provided that the bus messages are persistent (via a provider for Otto or sticky events for EventBus), you will not lose any messages this way.

Einberger

Answer 4

It's not really safe to have an @Produce on a Fragment, because more than one instance of the fragment can exist (and be registered on the bus) at the same time.

In my opinion @Produce really only makes sense on a singleton.

Cryptogenic