Image downloader with auto memory cleaning

Question

I have a list (simple ListBox) of items with images on master-detail base (if user clicks on list item, details page is opened). I faced quite famous problem with images memory leaks, described here, here, here, and here.

One possible way is to run through all images when NavigatingFrom and clean them.

In one of the threads, i found more interesting solution: it cleans images automatically, but that is not working for virtualization (images are lost or mixed, if to add private field for storing ImageSource). Suggested fix was to add dependency property.

But i'm still facing the same problem: images are mixed up after scrolling down and returning up. Looking like dependency property are changed randomly, but i cant catch the moment when they are changing.

public class SafePicture : ContentControl
{
    public static readonly DependencyProperty SafePathProperty =
        DependencyProperty.RegisterAttached(
            "SafePath",
            typeof(string),
            typeof(SafePicture),
            new PropertyMetadata(OnSourceWithCustomRefererChanged));

    public string SafePath
    {
        get { return (string)GetValue(SafePathProperty); }
        set { SetValue(SafePathProperty, value); }
    }

    private static void OnSourceWithCustomRefererChanged(DependencyObject o, DependencyPropertyChangedEventArgs e)
    {
        if (e.NewValue == null) // New value here
            return;
    }


    public SafePicture()
    {
        Content = new Image();
        Loaded += OnLoaded;
        Unloaded += OnUnloaded;
    }

    private void OnLoaded(object _sender, RoutedEventArgs _routedEventArgs)
    {
        var image = Content as Image;

        if (image == null)
            return;

        var path = (string)GetValue(SafePathProperty); // Also, tried SafePath (debugger cant catch setter and getter calls), but same result.

        image.Source = null;
        {
            var request = WebRequest.Create(path) as HttpWebRequest;
            request.AllowReadStreamBuffering = true;
            request.BeginGetResponse(result =>
            {
                try
                {
                    Stream imageStream = request.EndGetResponse(result).GetResponseStream();
                    DispatcherHelper.CheckBeginInvokeOnUI(() =>
                    {
                        if (imageStream == null)
                        {
                            image.Source = new BitmapImage { UriSource = new Uri(path, UriKind.Relative) };
                            return;
                        }

                        var bitmapImage = new BitmapImage();
                        bitmapImage.CreateOptions = BitmapCreateOptions.BackgroundCreation;
                        bitmapImage.SetSource(imageStream);
                        image.Source = bitmapImage;
                    });
                }
                catch (WebException)
                {
                }
            }, null);
        }
    }


    private void OnUnloaded(object sender, RoutedEventArgs e)
    {
        var image = Content as Image;

        if (image == null)
            return;

        var bitmapImage = image.Source as BitmapImage;
        if (bitmapImage != null)
            bitmapImage.UriSource = null;
        image.Source = null;
    }
}

Usage:

<wpExtensions:SafePicture SafePath="{Binding ImageUrl}"/>

So, at first glance, it works fine, but if to scroll down and return up, images are changed randomly.

EDIT: in this case, for now, i'm using only pure ListBox, without virtualization (but expecting it in other cases).

EDIT2: sample project to reproduce this problem. I believe, it would contain solution in a while: https://simca.codeplex.com/

Subantarctic

Answer 1

The problem is that when using virtualisation, the ui element used for each item are recycled and reused for other object (so that include the image object) and since you are loading the image asynchronously when you scroll fast enough your will be setting a Bitmap on an Image which are already been reused for another item.
A quick fix for is just to check that the path value is still the same and if it is not, just return since the image has already been reused for another object:

...
var request = WebRequest.Create(path) as HttpWebRequest;
        request.AllowReadStreamBuffering = true;
        request.BeginGetResponse(result =>
        {

            try
            {

                Stream imageStream = request.EndGetResponse(result).GetResponseStream();
                DispatcherHelper.CheckBeginInvokeOnUI(() =>
                {
                if (path!=SafePath){
                  //Item has been recycled
                  return;
                }
                 ....

Edit: There were a several problem in the code: -Switch RegisterAttached to Register, RegisterAttached is for attached property not normal dependency property -call OnLoaded in OnSourceWithCustomRefererChanged because the SafePath property changed can actually happpen after the element is loaded -Add clear uri and source at the start of onLoaded so that it clear image when path is empty

Here is a full working code :

public class SafeImage : ContentControl
{
    private SynchronizationContext uiThread;

    public static readonly DependencyProperty SafePathProperty =
        DependencyProperty.Register("SafePath", typeof (string), typeof (SafeImage),
        new PropertyMetadata(default(string), OnSourceWithCustomRefererChanged));

    public string SafePath
    {
        get { return (string) GetValue(SafePathProperty); }
        set { SetValue(SafePathProperty, value); }
    }


    private static void OnSourceWithCustomRefererChanged(DependencyObject o, DependencyPropertyChangedEventArgs e)
    {
        SafeImage safeImage = o as SafeImage;
        safeImage.OnLoaded(null, null);
        //OnLoaded(null, null);
        if (e.NewValue == null)
            return;
    }





    public SafeImage()
    {
        Content = new Image();
        uiThread = SynchronizationContext.Current;

        Loaded += OnLoaded;
        Unloaded += OnUnloaded;
    }

    private void OnLoaded(object _sender, RoutedEventArgs _routedEventArgs)
    {
        var image = Content as Image;

        if (image == null)
            return;

        var path = SafePath; //(string)GetValue(SafePathProperty);
        //image.Source = new BitmapImage(new Uri(SafePath));
        Debug.WriteLine(path);

        var bitmapImage = image.Source as BitmapImage;
        if (bitmapImage != null)
            bitmapImage.UriSource = null;
        image.Source = null;

        if (String.IsNullOrEmpty(path))
        {
            //image.Source = new BitmapImage { UriSource = new Uri(Constants.RESOURCE_IMAGE_EMPTY_PRODUCT, UriKind.Relative) };
            return;
        }

        // If local image, just load it (non-local images paths starts with "http")
        if (path.StartsWith("/"))
        {
            image.Source = new BitmapImage { UriSource = new Uri(path, UriKind.Relative) };
            return;
        }



        {
            var request = WebRequest.Create(path) as HttpWebRequest;
            request.AllowReadStreamBuffering = true;
            request.BeginGetResponse(result =>
            {
                try
                {
                    Stream imageStream = request.EndGetResponse(result).GetResponseStream();
                    uiThread.Post(_ =>
                    {

                        if (path != this.SafePath)
                        {
                            return;
                        }
                        if (imageStream == null)
                        {
                            image.Source = new BitmapImage { UriSource = new Uri(path, UriKind.Relative) };
                            return;
                        }

                        bitmapImage = new BitmapImage();
                        bitmapImage.CreateOptions = BitmapCreateOptions.BackgroundCreation;
                        bitmapImage.SetSource(imageStream);
                        image.Source = bitmapImage;
                        //imageCache.Add(path, bitmapImage);
                    }, null);
                }
                catch (WebException)
                {
                    //uiThread.Post(_ =>
                    //{
                    //    image.Source = new BitmapImage { UriSource = new Uri(Constants.RESOURCE_IMAGE_EMPTY_PRODUCT, UriKind.Relative) };
                    //}, null);
                }
            }, null);
        }
    }


    private void OnUnloaded(object sender, RoutedEventArgs e)
    {
        var image = Content as Image;

        if (image == null)
            return;

        var bitmapImage = image.Source as BitmapImage;
        if (bitmapImage != null)
            bitmapImage.UriSource = null;
        image.Source = null;
    }
}

Just as a final note, the windows phone ListBox is using virtualisation and recycling by default (the ItemPanel used is a VirtualisedStackPanel).

Inconclusive

Thanks for your help! I wish, i can send a beer to you :). However, problem is still there. (path!=SafePath) after BeginGetResponse() causes access Exception. And images are still mixed up (even with path check in Dispatcher) – Subantarctic

Anyway, thanks for highlighting reason. Another problem: i added breakpoints in SafePath's getter/setter, but debugger never stops there. Also, actually, this is the simplest case: i'm using it for the common ListBox, even without virtualization (unless there's some internal one). – Subantarctic

Uploading sample project. – Subantarctic

@VitaliiVasylenko the check just after BeginGetResponse was actually not needed, it was just a small optimisation but that not the important one, the one which need to be there is the one in CheckBeginInvokeOnUI just before creating the BitmapImage – Inconclusive

Regarding the fact that the debugger is not breaking, it's normal that it don't break in the setter, when using binding it don't actually use this setter to set the property (but it will call OnSourceWithCustomRefererChanged) but the getter should obviously be called when you execute if (path!=SafePath) – Inconclusive

Yep, but the problem is still there. See yourself: simca.codeplex.com Oops, forgot to add your suggestion to the sample, but result would be the same. – Subantarctic

ok updated my response with a working code, let me know if that work for you – Inconclusive

//the windows phone ListBox is using virtualisation and recycling by default (the ItemPanel used is a VirtualisedStackPanel). - quite strange, because if i have ListBox with 120-150 items (image + 3-5 textboxes), it slows system a lot, especially on the old phones like lumia 800. Anyway, thanks for detailed answer, let me see.. – Subantarctic

Hell yeah, seems to fork pretty fine. Your help is priceless.. do you have a project so i can support it? :) – Subantarctic