0

I want to generate List<Task> and then call it all parallel. I try to do this, but not understood how pass index in lambda expression. In my realisation always send last index of for. But i want to use all indexes in parallel computing...

List<Task> tasks = new List<Task>();

var  valueSize = Values.Count;
for (int i = 1; i <= valueSize; i++)
{
   tasks.Add(Task.Factory.StartNew(
   () => {
      this.Values[i] = this._tcpRepository.GetValueAsync(i).ToString().GetInt32();
   }));
}
Task.WaitAll(tasks.ToArray());

Additional information:

public Dictionary<int, int> Values { get; set; }

In TcpRepository

public async Task<string> GetValueAsync(int digit)
{
   var netStream = this.TcpClient.GetStream();
   if (netStream.CanWrite)
   {
      Byte[] sendBytes = Encoding.UTF8.GetBytes(digit.ToString());
      await netStream.WriteAsync(sendBytes, 0, sendBytes.Length);
   }
   if (netStream.CanRead)
   {
      byte[] bytes = new byte[this.Client.ReceiveBufferSize];

      await netStream.ReadAsync(bytes, 0, (int)this.Client.ReceiveBufferSize);
      return bytes.ToString();
   }
   return null;
}

GetInt32() it's my custom extension for string public static int GetInt32(this string value). Strings can come with garbage as characters that are not numbers.

Claus Stolz
  • 358
  • 4
  • 18
  • 4
    Do `int iCopy = i;` and pass `iCopy` to the lambda. See https://stackoverflow.com/questions/235455/access-to-modified-closure – Matthew Watson Sep 24 '20 at 08:51
  • 1
    Also `this._tcpRepository.GetValueAsync(i).ToString()` looks a bit suspicious. I don't think this will act as you like it to act. This will return the string representation of the `Task` object. Not the result of the method. Instead use this: `(await this._tcpRepository.GetValueAsync(i)).GetInt32();` to get the result returned by the method. – croxy Sep 24 '20 at 09:03
  • Related: [Captured variable in a loop in C#](https://stackoverflow.com/questions/271440/captured-variable-in-a-loop-in-c-sharp) – Theodor Zoulias Sep 24 '20 at 10:02

1 Answers1

4

There are a few issues here.

To directly answer your question, you need to create a closure over i to prevent it being updated before it is accessed in your async code.

You can do this by swapping your for loop with Enumerable.Range.

Another issue is that you are running GetValueAsync on the ThreadPool but not awaiting it. Subsequently, your Task.WaitAll will be waiting for the outer Tasks only, not the Tasks returned by GetValueAsync.

Here is an example of what you could do:

var tasks = Enumerable.Range(0, valueSize)
    .Select(async i =>
    {
        string val = await _tcpRepository.GetValueAsync(i);
        Values[i] = val.GetInt32();
    });

Your final issue is the use of Task.WaitAll; this introduces the sync-over-async antipattern. You should allow async to grow through your code base, and instead use Task.WhenAll, which returns a Task that completes when all the provided Tasks are complete:

await Task.WhenAll(tasks);
Johnathan Barclay
  • 18,599
  • 1
  • 22
  • 35