Memory leaks in BlockingBlurController

[alaegin]

Hello, @Dimezis

I'm using your librray in such way: Activity -> Fragment -> RecyclerView Item with blurred background.
So, when I replace fragment with recyclerview and blured items to another one I'm getting several leaks detected by LeakCanary.

screenshot #1
screenshot #2
screenshot #3

In my (and LeakCanary's) opinion the leak was caused by anonymous class in BlockingBlurController#deferBitmapCreation (read here about it)

I tried to remove it at all and as result no leaks detected and lower memory consumption achieved. So I can suggest to make that class static.

private static class SizeHandler implements ViewTreeObserver.OnGlobalLayoutListener {

        private WeakReference<View> blurView;
        private SizeHandlerListener listener;

        SizeHandler(View blurView, SizeHandlerListener listener) {
            this.blurView = new WeakReference<>(blurView);
            this.listener = listener;
        }

        @Override
        public void onGlobalLayout() {
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
                blurView.get().getViewTreeObserver().removeOnGlobalLayoutListener(this);
            } else {
                legacyRemoveOnGlobalLayoutListener();
            }

            int measuredWidth = blurView.get().getMeasuredWidth();
            int measuredHeight = blurView.get().getMeasuredHeight();

            listener.onSizeClarified(measuredWidth, measuredHeight);
        }

        @SuppressWarnings("deprecation")
        void legacyRemoveOnGlobalLayoutListener() {
            blurView.get().getViewTreeObserver().removeGlobalOnLayoutListener(this);
        }
    }

    private interface SizeHandlerListener {
        void onSizeClarified(int measuredWidth, int measuredHeight);
    }

And call it from function like it:

private void deferBitmapCreation() {
     new SizeHandler(blurView, this::init);
}

This solution fixed memory leak for me.

Regards,
Alexey Egin.

[Dimezis]

Hm, that's weird. I'm removing this listener straight away after onGlobalLayout.
There can be no leak, unless onGlobalLayout is not called at all.
Could you debug if it's called in your case?

[alaegin]

@Dimezis No, that method not called at all (I could remove it w/o any negative effect)

[Dimezis]

Weird stuff. How does BlurView initialize the bitmap in this case?
Also what's your device and OS?

[alaegin]

As I can see - deferBitmapCreation called only when measuredWidth and measuredHeight give us a zero sized downscaled image, probably in my case its not getting zero sized image (Correct if i'm wrong).

if (isZeroSized(measuredWidth, measuredHeight)) {
    deferBitmapCreation();
    return;
}

OS: Android 8.1
Device: OnePlus 5

[Dimezis]

But if it wasn't called, there would be no leak :)
Could you please debug why is this exactly happening?

Something similar to your approach would work, but I don't like using similar WeakReference-based solutions, because we have a legal way of avoiding a leak by removing a listener.
So we just need to make sure it's always properly called.

[alaegin]

I'm sorry: I missed some important thing.

It turned out that the deferBitmapCreation method called only on first few view bindings in RecyclerView Adapter. After that adapter reuses previously inflated views as it should be, without initializing new ones. Following logic we've initialized that views and listeners were removed - no leak shoud occur, but it occurs, although no new BlurController were created.

So far I can not understand why there is a leak...

[Dimezis]

Btw are you sure you don't have leaks anymore? Because essentially you're leaking it again by doing this::init.
Also, please don't ever use weak ref like this blurView.get().getViewTreeObserver().
get() can return null any time, so you have to make a check

[masc3d]

the massive memory leak Im experiencing in BlockingBlurController recently is actually caused by posting in setBlurAutoUpdate, which is called when blur view is detached and essential for removing the tree observer.

since post is never really reliable, least during view destruction, it will lead to tree observer usually not unregistering, leaking the controller and entire view hierarchy of the root view with it.

I don't know why it was introduced, but removing all posts in controller works fine and resolves leakage for me.

[masc3d]

In my (and LeakCanary's) opinion the leak was caused by anonymous class in BlockingBlurController#deferBitmapCreation (read here about it)

leak canary may report this as it may be GC'ed lazily but not every anonymous class is automatically an issue, despite leak canary (temporarily) reporting it.

I wouldn't see that ViewTreeObserver.OnGlobalLayoutListener is a real problem here as long as it's unregistered reliably at some point.

[masc3d]

@DominuS-RU could you check if you still see leakage with patch com.github.masc3d:blurview:17c1cdc48c

it requires repositories { maven { url 'https://jitpack.io' } }

[alexandrius]

@DominuS-RU @masc3d Can confirm the patched version above doesn't have the leak.

[masc3d]

@Dimezis do you prefer pull request for this?

[Dimezis]

@masc3d would really appreciate it