C# Parallel.ForEach and Task.WhenAll sometimes returning less values then supposed

Question

I have this:

Parallel.ForEach(numbers, (number) =>
{
    var value = Regex.Replace(number, @"\s+", "%20");

    tasks.Add(client.GetAsync(url + value));
});

await Task.WhenAll(tasks).ConfigureAwait(false);

foreach (var task in tasks)
{
  ...
}

Sometimes returns less tasks when reaching the foreach(var task in tasks), but after a few requests, starts returning all the tasks.

Ive changed the ConfigureAwait to true and still sometimes returns less tasks.

BTW Im using Parallel.ForEach beacuse each client.GetAsync(url + value) its a request to an external api with the particularity that its latency SLA is lower than 1s for 99% of its requests

Can you guys explain me why it returns less tasks sometimes?

And is there a way to guarantee returning always all tasks?

Thanks

Answer 1

You haven't shown it, so we can only guess but I assume that tasks is a List<>. This collection type is not thread-safe; your parallel loop is likely "overwriting" values. Either perform manual locking of your list or switch to a thread-safe collection such as a ConcurrentQueue<>

var tasks = new ConcurrentQueue<Task<string>>();

Parallel.ForEach(numbers, number =>
{
    var value = Regex.Replace(number, @"\s+", "%20");
    tasks.Enqueue(client.GetAsync(url + value));
});

await Task.WhenAll(tasks.ToArray()).ConfigureAwait(false);

foreach (var task in tasks)
{
   // whatever 
}

That said, your use of Parallel.ForEach is quite suspect. You aren't performing anything of real significance inside the loop. Use of Parallel, especially with proper locking, likely has higher overhead negating any potential gains you claim to observe or are realized by paralellizing the Regex calls. I would convert this to a normal foreach loop and precompile the Regex to offset (some of) its overhead:

// in class
private static readonly Regex SpaceRegex = new Regex(@"\s+", RegexOptions.Compiled);

// in method
var tasks = new List<Task<string>>();

foreach (var number in numbers)
{
    var value = SpaceRegex.Replace(number, "%20");
    tasks.Add(client.GetAsync(url + value));
}

await Task.WhenAll(tasks).ConfigureAwait(false);

foreach (var task in tasks)
{
   // whatever 
}

Alternatively, don't use a regex at all. Use a proper Uri escaping mechanism which will have the added benefit of fixing more than just spaces:

var value = Uri.EscapeDataString(number);
// or
var fullUri = Uri.EscapeUriString(url + number);

Note there are two different methods there. The proper one to use depends on the values of url and number. There's also other mechanisms such as the HttpUtility.UrlEncode method... but I think these are the preferred ones.