2

I'm trying to implement multi-threading as suggested here: Spawn Multiple Threads for work then wait until all finished

Code looks like so:

var resetEvent = new ManualResetEvent(false);
            var clientsCount = IDLocal.UserSessions.Count;

            // Load sessions:
            IDLocal.UserSessions = new IDSessions();

            // Start thread for each client 
            foreach (IDSession session in IDLocal.UserSessions)
            {
                var clnt = session;
                new Thread(
                    delegate()
                    {
                        Debug.WriteLine(Thread.CurrentThread.ManagedThreadId);
                        clnt.FailedToRespond = !this.SendDirect(data, this.GetHostAddress(clnt.HostName), clnt.PortNumber);

                        // If we're the last thread, signal 
                        if (Interlocked.Decrement(ref clientsCount) == 0) resetEvent.Set();
                    })
                    .Start();
            }

Here I'm getting ReSharper warning: if (Interlocked.Decrement(ref clientsCount) == 0)

It suggests that I'm accessing modified closure (clientsCount)

Is that a valid suggestion?

Community
  • 1
  • 1
katit
  • 17,375
  • 35
  • 128
  • 256
  • 1
    Yes it is a valid suggestion. mutable closed over variables should be avoided. – Lukasz Madon Sep 20 '12 at 21:49
  • Isn't this an exception to the rule, as he is mutating a variable whose value is meant to be shared across threads? – dugas Sep 20 '12 at 21:56
  • 3
    @dugas Yes, this is not an example of what the warning is about, so the warning should be ignored here. The warning is about capturing variables without realising that their values change. In this case, you want the values to change. –  Sep 20 '12 at 21:58

1 Answers1

3

The warning is meant to cover code such as

int i = 1;
Func<int> f = () => i + 1;
i = 2;
int j = f(); // What does this set j to?

where the updated value of i will be used when calling f. The warning suggests to change this to

int i = 1;
int icopy = i;
Func<int> f = () => icopy + 1;
i = 2;
int j = f(); // Now what does this set j to?

if you want f to see the value of i as it was when the delegate was created. In the context of foreach loops, it is very easy to otherwise get unintuitive behaviour. There has been a change in the meaning of foreach loops in C# 5 because of exactly this.

Since that's not what you want here, since you do want to see changes to the captured variable, don't change anything in the code.

  • I thought so. Espacially that it refers to variable by `ref` Resharper should be smarter than that.. – katit Sep 20 '12 at 22:11