FragmentManager(v4) not removing fragments from mCreatedMenus

Question

LeakCanary identified leak in my code

* classifieds.yalla.features.ad.page.seller.SellerAdPageFragment has leaked:
* GC ROOT android.view.inputmethod.InputMethodManager$1.this$0 (anonymous subclass of com.android.internal.view.IInputMethodClient$Stub)
* references android.view.inputmethod.InputMethodManager.mNextServedView
* references android.support.v4.widget.DrawerLayout.mContext
* references classifieds.yalla.features.host.HostActivity.fragNavController
* references com.ncapdevi.fragnav.FragNavController.mFragmentManager
* references android.support.v4.app.FragmentManagerImpl.mCreatedMenus
* references java.util.ArrayList.elementData
* references array java.lang.Object[].[0]
* leaks classifieds.yalla.features.ad.page.seller.SellerAdPageFragment instance

But when I looked in the FragmentManagerImpl

I didn't found when FragmentManagerImpl.mCreatedMenus getting cleared. The only code I found is when new fragments getting added. Shouldn't it be managed somehow?

public boolean dispatchCreateOptionsMenu(Menu menu, MenuInflater inflater) {
        boolean show = false;
        ArrayList<Fragment> newMenus = null;
        if (mAdded != null) {
            for (int i=0; i<mAdded.size(); i++) {
                Fragment f = mAdded.get(i);
                if (f != null) {
                    if (f.performCreateOptionsMenu(menu, inflater)) {
                        show = true;
                        if (newMenus == null) {
                            newMenus = new ArrayList<Fragment>();
                        }
                        newMenus.add(f);
                    }
                }
            }
        }

        if (mCreatedMenus != null) {
            for (int i=0; i<mCreatedMenus.size(); i++) {
                Fragment f = mCreatedMenus.get(i);
                if (newMenus == null || !newMenus.contains(f)) {
                    f.onDestroyOptionsMenu();
                }
            }
        }

        mCreatedMenus = newMenus;

        return show;
    }

Answer 1

This issue is still relevant nowadays on androidx.fragment v1.10 (so Nov. 2019), so here's some insight to it.

Assume setHasOptionsMenu() is called with a true value for a fragment f. When f is detached, the Fragment Manager (FM) associated with f will not handle the change implied on the menu. Remember that the menu is potentially affected by multiple fragments hosted by the same FM. The fact that one of them, f, got detached should have caused the FM to reconstruct the menu, but then again, this is not handled. Further more, when f is detached, resources associated with f in the context of supporting the menu are also not cleaned up. In particular, onDestroyOptionsMenu() is not called on f and the FM keeps a reference to f in its list of fragments that provide menu options.

Until Google fixes the fragment manager to remove the leaked fragment from that list, some options are:

• Live with the leaked fragment. When the activity gets destroyed, the fragment manager will be cleared out and the fragment will get claimed by the GC then.
• Don't use the setHasOptionsMenu() mechanism. You could for example, come up with your own menu implemenation.
• Use reflection to remove the fragment from that list. Sure reflection is not ideal, but leaking a fragment is much worse. In the otherwise leaking fragment, add something like the following

@Override
public void onDetach() {
    super.onDetach();

    // get the fragment manager associated with this fragment
    FragmentManager fragmentManager = getFragmentManager();
    if (fragmentManager != null) {
        try {
            Field field = 
                fragmentManager.getClass().getDeclaredField("mCreatedMenus");
            field.setAccessible(true);

            if (field.get(fragmentManager) instanceof ArrayList) {
                ArrayList fragments = (ArrayList)field.get(fragmentManager);

                if (fragments != null && fragments.remove(this)) {
                    Log.d(TAG, "Yay, no leak today");
                }
            }
        } catch (NoSuchFieldException | SecurityException | 
                 IllegalAccessException e) {
            e.printStackTrace();
        }
    }
}

Note: Naturally, this solution is fragile when fragment related code changes, however, this is testable. In addition if proguard is used, you would want to make sure obfuscation is avoided for that field, so you could add proguard directive like this:

-keep class androidx.fragment.app.FragmentManagerImpl { *; }

Or even better, try to figure out how to use -keepclassmembers for keeping mCreatedMenus.