gowong / material-sheet-fab

Android library that provides the floating action button to sheet transition from Google's Material Design.
MIT License
1.58k stars 255 forks source link

Memory Leak in MaterialSheetFab #21

Closed agrosner closed 8 years ago

agrosner commented 8 years ago

The call:

// Set listener for when FAB view is laid out
        fab.getViewTreeObserver()
                .addOnGlobalLayoutListener(new ViewTreeObserver.OnGlobalLayoutListener() {
                    @Override
                    public void onGlobalLayout() {
                        // Initialize FAB anchor when the FAB view is laid out
                        if (!isFabLaidOut) {
                            updateFabAnchor();
                            isFabLaidOut = true;
                        }
                    }
                });

appears to leak the instance to this sheet (causing my activity to leak), since it's not removed from the OnGlobalLayoutListener. This was caught using LeakCanary.

gowong commented 8 years ago

Can you include the leak trace?

agrosner commented 8 years ago
in $$:4.03.00:40300.
* $$.databinding.FragmentCategoryGridBinding has leaked:
* GC ROOT static android.os.UserManager.sInstance
* references android.os.UserManager.mContext
* references android.app.ContextImpl.mOuterContext
* references $$.shop.ShopActivity.mDecor
* references com.android.internal.policy.impl.PhoneWindow$DecorView.mAttachInfo
* references android.view.View$AttachInfo.mTreeObserver
* references android.view.ViewTreeObserver.mOnGlobalLayoutListeners
* references android.view.ViewTreeObserver$CopyOnWriteArray.mData
* references java.util.ArrayList.array
* references array java.lang.Object[].[0]
* references com.gordonwong.materialsheetfab.MaterialSheetFab$3.this$0 (anonymous class implements android.view.ViewTreeObserver$OnGlobalLayoutListener)
* references com.gordonwong.materialsheetfab.MaterialSheetFab.overlayAnimation
* references com.gordonwong.materialsheetfab.animations.OverlayAnimation.overlay
* references com.gordonwong.materialsheetfab.DimOverlayFrameLayout.mParent
* references android.widget.RelativeLayout.mKeyedTags
* references android.util.SparseArray.mValues
* references array java.lang.Object[].[0]
* leaks $$.databinding.FragmentCategoryGridBinding instance

* Reference Key: 2dc10c72-9c34-4068-bcb7-095d48c3d10d
* Device: samsung samsung SAMSUNG-SM-G920A zeroflteuc
* Android Version: 5.1.1 API: 22 LeakCanary: 1.3.1
* Durations: watch=5391ms, gc=220ms, heap dump=3666ms, analysis=20230ms
agrosner commented 8 years ago

fixed it by moving the OnGlobalLayoutListener into a local variable:

 private final ViewTreeObserver.OnGlobalLayoutListener onGlobalLayoutListener = new ViewTreeObserver.OnGlobalLayoutListener() {
        @Override
        public void onGlobalLayout() {
            // Initialize FAB anchor when the FAB view is laid out
            if (!isFabLaidOut) {
                updateFabAnchor();
                isFabLaidOut = true;
            }
        }
    };

and then modifying the instantiation:

// Set listener for when FAB view is laid out
        fab.getViewTreeObserver()
                .addOnGlobalLayoutListener(onGlobalLayoutListener);

and then creating a dispose() method to deregister it here:

/**
     * Cleans up this view (if any).
     */
    public void dispose() {
        fab.getViewTreeObserver().removeOnGlobalLayoutListener(onGlobalLayoutListener);
    }
gowong commented 8 years ago

Nice! Feel free to submit a pull request if you'd like. Otherwise, I'll make the change when I get the chance.