7

I am adding Background Tasks to a Blocking Collection (added in the Background).

I am waiting with Task.WhenAll on a Enumerable returned by GetConsumingEnumerable.

My question is: Is the overload of Task.WhenAll which receives an IEnumerable "prepared" to potentially receive an endless amount of tasks?

I am simply not sure if i can do it this way or if it was meant to be used this way?

private async Task RunAsync(TimeSpan delay, CancellationToken cancellationToken)
{
    using (BlockingCollection<Task> jobcollection = new BlockingCollection<Task>())
    {
        Task addingTask = Task.Run(async () =>
        {
            while (true)
            {
                DateTime utcNow = DateTime.UtcNow;
                var jobs = Repository.GetAllJobs();
                foreach (var job in GetRootJobsDue(jobs, utcNow))
                {
                    jobcollection.Add(Task.Run(() => RunJob(job, jobs, cancellationToken, utcNow), cancellationToken), cancellationToken);
                }

                await Task.Delay(delay, cancellationToken);
            }
        }, cancellationToken);

        await Task.WhenAll(jobcollection.GetConsumingEnumerable(cancellationToken));
    }
}
i3arnon
  • 113,022
  • 33
  • 324
  • 344
rudimenter
  • 3,242
  • 4
  • 33
  • 46
  • Seems like using `Parallel.Foreach(Repository.GetAllJobs,....)` would be a simpler solution. – L.B Aug 25 '14 at 14:51
  • `Task.WhenAll` will block the calling thread till `GetConsumingEnumerable` finishes – Sriram Sakthivel Aug 25 '14 at 14:53
  • Well if there are an infinite number of tasks they'll never all finish, because you'll never get the whole set. – Servy Aug 25 '14 at 14:53
  • @SriramSakthivel GetConsumingEnumerable returns immediatelly an IEnumerable but only gets iterated when data is available... that intended behaviour – rudimenter Aug 25 '14 at 14:55
  • @rudimenter I edited my comment before you reply. See if that makes sense? – Sriram Sakthivel Aug 25 '14 at 14:57
  • @Servy Thats what i want... Only return when CancellationToken is fired... (deep in some other code) – rudimenter Aug 25 '14 at 14:57
  • @rudimenter But `WhenAll` doesn't yield control until it has been able to enumerate all of the items in the sequence it's given. – Servy Aug 25 '14 at 14:58
  • @rudimenter Then don't use `WhenAll` at all, just wait on that cancellation token. – Servy Aug 25 '14 at 14:59
  • @L.B Nope because you still need to add the task to the Enumerable... but yes as a replacement for WhenAll – rudimenter Aug 25 '14 at 14:59
  • It's not even going to execute that line until the token has been cancelled... if you want to return when it gets cancelled, just remove that line altogether. – Kyle W Aug 25 '14 at 15:10

3 Answers3

7

Since your goal is merely to wait until the cancellation token is cancelled, you should do that. For reasons others have explained, using WhenAll on an infinite sequence of tasks isn't the way to go about that. There are easier ways to get a task that will never complete.

await new TaskCompletionSource<bool>().Task
    .ContinueWith(t => { }, cancellationToken);
Community
  • 1
  • 1
Servy
  • 202,030
  • 26
  • 332
  • 449
  • It seems his goal is to wait until all the tasks have been completed, not until they've all been created. – Kyle W Aug 25 '14 at 15:04
  • How does that work? `TaskCompletionSource()Task` is never going to complete which means `ContinueWith` never going to execute. I assume you mean to call `SetCancelled` on the Task? – Sriram Sakthivel Aug 25 '14 at 15:05
  • @KyleW [No, it's not.](http://stackoverflow.com/questions/25488574/usage-of-task-whenall-with-infinite-tasks-produced-by-blockingcollection/25488865?noredirect=1#comment39782016_25488574) – Servy Aug 25 '14 at 15:05
  • @SriramSakthivel Nope, this will work as is. When the token is cancelled it'll stop waiting for the never ending task it is a continuation of to finish. Your approach would work to, it'd just be more lines of code. – Servy Aug 25 '14 at 15:06
  • I see, That's beautiful. I overlooked it.. +1 – Sriram Sakthivel Aug 25 '14 at 15:11
  • This would render also the BlockingCollection useless... I could simply run the Tasks in the loop in the background and just await addingTask – rudimenter Aug 25 '14 at 15:18
5

Task.WhenAll will not work with an infinite number of tasks. It will first (synchronously) wait for the enumerable to finish, and then (asynchronously) wait for them all to complete.

If you want to react to a sequence in an asynchronous way, then you need to use IObservable<Task> (Reactive Extensions). You can use a TPL Dataflow BufferBlock as a "queue" that can work with either synchronous or asynchronous code, and is easily convertible to IObservable<Task>.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Does the same apply for Parallel.Foreach. Will it also wait until the enumeration is finished? – rudimenter Aug 25 '14 at 15:07
  • @rudimenter `Parallel.Foreach` is an inherently synchronous operation, not an asynchronous one. Not only will it synchronously wait for the sequence to finish, it'll synchronously wait to finish processing all of the items. – Servy Aug 25 '14 at 15:09
  • I'm pretty sure that `Parallel` will not immediately wait for the enumeration before starting processing, but you will "lose" a thread to the enumerating. – Stephen Cleary Aug 25 '14 at 15:09
0

I assume that Task.WhenAll will try to enumerate the collection, which means that it itself will block until the collection is completed or cancelled. If it didn't, then the code could theoretically finish awaiting before the tasks have been created. So there's going to be an extra block in there... it will block waiting for the threads to be created, and then block again until the tasks are done. I don't think that's a bad thing for your code, as it's still going to block until the same point in time.

Kyle W
  • 3,702
  • 20
  • 32