-1

Alright, so I'm having a bit of an issue here. Here is the loop.

lock (ClientLocker)
{
    Trace.WriteLine("#WriteAll: " + sm.Header);
    foreach (Client c in Clients)
    {
        if (c.LoggedIn)
        {
            Trace.WriteLine("#TryWriteTo[" + c.Id + "](" + sm.Header + ")");
            LazyAsync.Invoke(() => c.WriteMessage(sm));
        }
    }
}

Here is LazyAsync

public static class LazyAsync
{
    public static void Invoke(Action a)
    {
        a.BeginInvoke(a.EndInvoke, null);
    }
}

Each Client contains a socket, so I can't hardly Clone it. The problem is, when I do the Invoke to c.WriteMessage, since the execution is delayed, it usually won't fire on the first couple in the list, and will sometimes actually only fire a whole bunch on the very last item.

I know this has to do with c being a reference that changes before Invoke actually gets called, but is there a way to avoid this?

Doing a general for(int i=0 etc loop doesn't seem to fix this issue.

Anyone have any ideas on how I can fix this?

Remember, can't Clone Client.

Kelly Elton
  • 4,373
  • 10
  • 53
  • 97
  • 3
    Someone asks this just about every day. See http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach/8899347#8899347 for some links and discussion about the issue. – Eric Lippert Jan 25 '12 at 20:14
  • I did a search, but didn't find anything. Not like I didn't try. – Kelly Elton Jan 25 '12 at 20:47
  • Indeed; this is a hard one to find. That is precisely why this question is asked again almost every day. Unless you actually get the "access to modified closure" warning from resharper, there's no reason why you would know what keywords to search on. – Eric Lippert Jan 25 '12 at 20:57
  • Yeah my trail for that ran out :p – Kelly Elton Jan 25 '12 at 21:07
  • possible duplicate of [C# Captured Variable In Loop](http://stackoverflow.com/questions/271440/c-sharp-captured-variable-in-loop) – nawfal Nov 02 '13 at 06:14

3 Answers3

5

Copy your c to local variable like this:

lock (ClientLocker)
{
    Trace.WriteLine("#WriteAll: " + sm.Header);
    foreach (Client c in Clients)
    {
        if (c.LoggedIn)
        {
            Client localC = c;
            Trace.WriteLine("#TryWriteTo[" + c.Id + "](" + sm.Header + ")");
            LazyAsync.Invoke(() => localC.WriteMessage(sm));
        }
    }
}

Do a web search for: "Access to modified closure" if you want to get more information.

CodingGorilla
  • 19,612
  • 4
  • 45
  • 65
  • Thank you, this seems like such an easy solution, and I feel a bit dumb that I didn't think of it. I guess I just assumed that the same thing would happen with the local variable as well. Thanks. – Kelly Elton Jan 25 '12 at 20:18
1

Your suspicion is right: the variable c is captured by the lambda expression, but not evaluated until later.

This flavor of error pops up whenever you make use of a loop variable within a lambda expression, since the loop variable is scoped outside the loop, and not with each iteration of the loop.

You can work around this by creating a new local variable in the foreach loop, assign c to it, and then pass that new local variable into the lambda expression:

lock (ClientLocker)
{
    Trace.WriteLine("#WriteAll: " + sm.Header);
    foreach (Client c in Clients)
    {
        if (c.LoggedIn)
        {
            Trace.WriteLine("#TryWriteTo[" + c.Id + "](" + sm.Header + ")");

            Client copyOfC = c;
            LazyAsync.Invoke(() => copyOfC.WriteMessage(sm));
        }
    }
}

Here are a few related StackOverflow posts:

Community
  • 1
  • 1
Matt Dillard
  • 14,677
  • 7
  • 51
  • 61
  • I appreciate the links, it'll be nice to read up on some more stuff. One of those grey lines where you're not sure what you're referencing :p – Kelly Elton Jan 25 '12 at 20:46
1

Try setting c to a local variable and calling LazyAsync.Invoke on that, to avoid c being reassigned to by the foreach loop before the invoke happens. When LazyAsync.Invoke does c.WriteMessage, it's calling WriteMessage on whatever c happens to now point to, not what it was when LazyAsync.Invoke(() => c.WriteMessage(sm)) was evaluated

foreach (Client c in Clients)
{
    if (c.LoggedIn)
    {
        Trace.WriteLine("#TryWriteTo[" + c.Id + "](" + sm.Header + ")");

        Client client = c;
        LazyAsync.Invoke(() => client.WriteMessage(sm));
    }
}
codekaizen
  • 26,990
  • 7
  • 84
  • 140
Bort
  • 7,398
  • 3
  • 33
  • 48