Access to modified closureSep6

Monday, 6 September 2010 by haemoglobin

Let’s play! Lately, I have been doing a fair bit of Silverlight development which forces an asynchronous style of development. After installing Resharper I would often see the message “Access to modified closure”. This happens when you have code that looks like this (note following is a contrived example but similar to the type of code you might end up writing when writing against asynchronous WCF services): 

   
class Program
    {
        class Person
        {
            public string Name;
            public string Greeting
            {
                set
                {
                    Console.WriteLine(String.Format("Setting {0} to {1}", Name, value));
                }
            }
        }
       
        static void Main(string[] args)
        {
            var peopleList = new List<Person>();
            peopleList.Add(new Person() { Name = "John Smith" });
            peopleList.Add(new Person() { Name = "Bobby Jones" });

            foreach (var person in peopleList)
            {
                GetGreeting(person, s => person.Greeting = s);
            }

            Console.ReadLine();
        }

        static void GetGreeting(Person person, Action<string> messageResponse)
        {
            messageResponse(String.Format("Hi, {0}!", person.Name.Split()[0]));
        }
    }

Resharper will actually underline the lambda expression above (s => person.Greeting = s) with the aforementioned message – sometimes this can be safe to ignore depending on what you are doing. In the above example what do you think the output should be?

No surprises actually:

image

But, some problems occur if the call to GetGreeting is asynchronous, as Silverlight callbacks to the server always are.

We simulate this by making GetGreeting asynchronous instead:

static void GetGreeting(Person person, Action<string> messageResponse)
{
    var worker = new BackgroundWorker();

    worker.DoWork += (s, e) =>
                         {
                             var parameters = (Tuple<Person, Action<string>>)e.Argument;
                             Person p = parameters.Item1;
                             Action<string> callback = parameters.Item2;

                             callback(String.Format("Hi, {0}!", p.Name.Split()[0]));
                         };

    worker.RunWorkerAsync(new Tuple<Person, Action<string>>(person, messageResponse));
}
 

Guess what we have now? Interesting to know that it will always be the following:

image

Notice that it is setting the two greetings on the same Bobby Jones object. Now that’s what Resharper is warning you about when it is saying “Access to modified closure” – to understand what is going on here I recommend reading the following post:
The implementation of anonymous methods in C# and its consequences

Essentially once you know how things work under the hood it makes perfect sense – the Person object is set as an instance field on a generated class that contains the anonymous method, and its reference is updated by the foreach loop rapidly to the last person object in the list, and then by the time the asynchronous method(s) have a chance to act on it, it ends up acting on the last object in the List twice.

In the application I am working on I need to pre-load a lot of data up front and have the UI display an in progress style loading panel. To solve the problem above, as well as not to flood the server with a tonne of requests all at once, I decided to load each object one at a time and display feed back. I could have used a Queue or an Enumerator for this, however I needed to display “Loading 3 of 34” type information, keeping the current index information outside of the Queue or Enumerator was messy and repeated, so ended up implementing a Iterator class that I can use for this (a quick search didn’t reveal anything like this but it was fast enough to make – not thread safe but suitable for my use): 

public class Iterator<T> where T : class
{
    private int _current = -1;
    public int CurrentIndex
    {
        get
        {
            return _current;
        }
    }

    public int Count
    {
        get
        {
            return _items.Count;
        }
    }

    readonly List<T> _items;
    public Iterator(List<T> items)
    {
        _items = items;
    }

    public T Next()
    {
        _current++;
        if (_current >= _items.Count)
            return null;
        else
        {
            return _items[_current];
        }
    }

    public T Current()
    {
        return _items[_current];
    }
}


class Program
{
    class Person
    {
        public string Name;
        public string Greeting
        {
            set
            {
                Console.WriteLine(String.Format("Setting {0} to {1}", Name, value));
            }
        }
    }
   
    static void Main(string[] args)
    {
        var peopleList = new List<Person>();
        peopleList.Add(new Person() { Name = "John Smith" });
        peopleList.Add(new Person() { Name = "Bobby Jones" });

        var peopleIterator = new Iterator<Person>(peopleList);
        LoadNextPersonGreeting(peopleIterator);
        Console.ReadLine();
    }

    static void LoadNextPersonGreeting(Iterator<Person> peopleIterator)
    {
        Person nextPerson = peopleIterator.Next();
        if (nextPerson != null)
        {
            Console.WriteLine("Loading {0} of {1}", peopleIterator.CurrentIndex + 1, peopleIterator.Count);
            GetGreeting(nextPerson, s =>
            {
                nextPerson.Greeting = s;
                LoadNextPersonGreeting(peopleIterator);
            });
        }
    }

    static void GetGreeting(Person person, Action<string> messageResponse)
    {
        //Same as before.
    }
}

image

I thought this was quite interesting and worthy of a blog post,

Thanks,
Hamish

Categories:   Development
Actions:   E-mail | Permalink | Comments
blog comments powered by Disqus

Powered by BlogEngine.NET 1.6.1.0 | Design by styleshout | Enhanced by GravityCube.net | 1.4.5 Changes by zembian.com | Adapted by HamishGraham.NET
(c) 2010 Hamish Graham. Banner Image (c) Chris Gin