C# Access to modified closure

Question

public virtual void OnRegistrationJoin(RegistrationJoinEventArgs e)
{
    foreach (Mobile member in e.Team)
    {
        member.SendMessage(1161, "You join the {0}.", EventFullName);

        if (e.Team.Count > 1)
        {
            Joinees.Remove(member);
            member.SendMessage(1161, "Your team formation is:");

            int i = 0;

            foreach (Mobile parter in e.Team.Where(partner => partner != member).ToList())
            {
                member.SendMessage(1150, "{0}: {1}.", ++i, partner.Name);
            }
        }
    }

    Members.Add(e.Team);
}

I get "access to modified closure" warning by resharper, I was wondering what's so wrong with this code, since all I do in the inner loop is send a message?

Possible duplicate of [Access to Modified Closure](http://stackoverflow.com/questions/235455/access-to-modified-closure) and several others. — adrianbanks, Apr 03 '11 at 00:13

codekaizen · Accepted Answer · 2022-03-25T21:51:36.107

15

The problem is in:

e.Team.Where(partner => partner != member)

The variable member is a direct reference to the member variable in the outer scope. While you might not have a problem with this in the above code, it is problematic when you are running the code on multiple threads or if you aren't evaluating the lambda in the Where method right away (for example, using IQueryable instead of IEnumerable).

The reason this is a problem is that C# generates a method to then pass as a delegate to Where. That method needs direct access to member. If you were to assign the reference to another variable like this:

var m = member;
// ...
e.Team.Where(partner => partner != m);

Then C# can "capture" that value in a construct called a "closure" and pass it to the generated method. This will ensure that when member changes, the value you expect it to be when you pass it to Where isn't changed.

edited Mar 25 '22 at 21:51

answered Apr 02 '11 at 23:48

codekaizen

26,990
7
84
140

+1: Ultimately the bigest problems that I see coming out of this for most people is deferred execution. – vcsjones Apr 02 '11 at 23:49
Easily fixed by doing var closureMember = member. And using closureMember in the LINQ. – Andrew T Finnell Apr 02 '11 at 23:50
@Femaref - but it's the changing reference that's the problem, not the changing instance. – codekaizen Apr 02 '11 at 23:53
Of course it is. But the behaviour of your `closureMember` will be exactly the same - each iteration, it will change to the current object. If he weren't using the query directly but deferred, only the last member of the foreach'ed sequence would be used. – Femaref Apr 02 '11 at 23:55
Oh, you meant assigning `closureMember` outside the lambda. Of course, that will work. Sorry. – Femaref Apr 02 '11 at 23:59
Thanks for the explaination, can you edit with a simple case example where this would actually become a problem? – bevacqua Apr 03 '11 at 00:38

score 2 · Answer 2 · answered Apr 02 '11 at 23:51

The part resharper complains about is e.Team.Where(partner => partner != member).ToList(), as the referenced member will be changed. In this case, this isn't a problem, but in other cases, this can be a problem.

Note: you don't have to use ToList(), which forces eager evaluation of the IEnumerable<T>. Simply iterate over e.Team.Where(partner => partner != member).

score 0 · Answer 3 · answered Apr 02 '11 at 23:47

0

I suppose member.SendMessage could modify member

That's modifying a closed variable used in the lambda

answered Apr 02 '11 at 23:47

sehe

374,641
47
450
633

Not really. The problem is the outer foreach loop which modifies the bound `member` variable in the lambda. – Femaref Apr 02 '11 at 23:53

C# Access to modified closure

3 Answers3