-2

I want to call an asynchronous method multiple times in a xUnit test and wait for all calls to complete before I continue execution. I read that I can use Task.WhenAll() and Task.WaitAll() for precisely this scenario. For some reason however, the code is deadlocking.

[Fact]
public async Task GetLdapEntries_ReturnsLdapEntries()
{
    var ldapEntries = _fixture.CreateMany<LdapEntryDto>(2).ToList();
    var creationTasks = new List<Task>();
    foreach (var led in ldapEntries)
    {
        var task = _attributesServiceClient.CreateLdapEntry(led);
        task.Start();
        creationTasks.Add(task);
    }
    Task.WaitAll(creationTasks.ToArray()); //<-- deadlock(?) here
    //await Task.WhenAll(creationTasks);

    var result = await _ldapAccess.GetLdapEntries();

    result.Should().BeEquivalentTo(ldapEntries);
}

public async Task<LdapEntryDto> CreateLdapEntry(LdapEntryDto ldapEntryDto)
{
    using (var creationResponse = await _httpClient.PostAsJsonAsync<LdapEntryDto>("", ldapEntryDto))
    {
        if (creationResponse.StatusCode == HttpStatusCode.Created)
        {
            return await creationResponse.Content.ReadAsAsync<LdapEntryDto>();
        }

        throw await buildException(creationResponse);
    }
}

The system under test is a wrapper around an HttpClient that calls a web service, awaits the response, and possibly awaits reading the response's content that is finally deserialized and returned.

When I change the foreach part in the test to the following (ie, don't use Task.WhenAll() / WaitAll()), the code is running without a deadlock:

foreach (var led in ldapEntries)
{
    await _attributesServiceClient.CreateLdapEntry(led);
}

What exactly is happening?

EDIT: While this question has been marked as duplicate, I don't see how the linked question relates to this one. The code examples in the link all use .Result which, as far as I understand, blocks the execution until the task has finished. In contrast, Task.WhenAll() returns a task that can be awaited and that finishes when all tasks have finished. So why is awaiting Task.WhenAll() deadlocking?

Thaoden
  • 3,460
  • 3
  • 33
  • 45
  • @Servy Thanks for the link, but I don't see the relation to `Task.WhenAll() / WaitAll()` ? – Thaoden Feb 19 '19 at 15:01
  • The duplicate explains exactly why the deadlock is happening, and what to do about it. Simply read through the answers. – Servy Feb 19 '19 at 15:08
  • That's what I just did before commenting and I still don't see the relation to `WaitAll` or `WhenAll`, the latter creating an awaitable task and still deadlocking; I don't use `.Result` anywhere. Telling me "Read it again" sadly won't help me. – Thaoden Feb 19 '19 at 15:13
  • Are you getting compiler warnings when you use Task.WaitAll? I suspect you are, since your method is marked as async. and it's not calling await. It's also not returning a Task, so it looks like it wouldn't even compile. Anyways, Task.WaitAll definitely shouldn't be used. And what kind of app is this (ASP.NET, ASP.NET Core, WPF, Win Forms etc...)? – mason Feb 19 '19 at 16:25
  • @mason Which one? All methods are returning `Task` or `Task`. `Task.WaitAll()` can't be awaited. I don't get any compiler warnings or errors, this code works fine (except for the deadlock, using the alternate `foreach` version works as intended). – Thaoden Feb 19 '19 at 16:28
  • GetLdapEntries_ReturnsLdapEntries return type is Task. But you're not returning a Task or awaiting anything. Which is why I believe it won't compile. And I know Task.WaitAll can't be awaited. That's why you shouldn't be using it. You also didn't answer my question about what platform you're using. – mason Feb 19 '19 at 16:30
  • @mason I use `await` in the `var result = await _ldapAccess.GetLdapEntries();` line. "Thats why you shouldn't be using it" (I guess this extends to `Task.WhenAll()`?) is exactly my question. – Thaoden Feb 19 '19 at 16:32
  • Ah yes, I missed that. So you won't get a compilation error. But anyways, I didn't say you shouldn't use Task.WhenAll. I said you shouldn't use Task.WaitAll. One of them is async and returns a Task, the other isn't and doesn't. – mason Feb 19 '19 at 16:34
  • @mason It doesn't make a difference if I use `WhenAll` or `WaitAll` - both of them are deadlocking and I don't understand why. Besides, not using `await` in an `async` method is not a compiler error, merely a compiler warning. – Thaoden Feb 19 '19 at 16:38
  • You should be treating most compiler warnings as errors: they indicate something seriously wrong with the code. And yes, I understand you're saying both are deadlocking. I didn't say switching to Task.WhenAll would fix it. I just said that Task.WaitAll is not the correct approach. Anyways, you still have yet to answer my question about what platform you're using. – mason Feb 19 '19 at 16:40
  • @mason I missed that question due to it being a later edit. The xUnit tests and the system under test both are .NET Core 2.2 libraries. Yes, you said `Task.WaitAll` is not the right approach, but the question why that is still remains. – Thaoden Feb 19 '19 at 16:54
  • Task.WaitAll can't be awaited. It's little different from `foreach(var task in tasks) { task.Wait(); }` so you throw all the advantages of async out the window. – mason Feb 19 '19 at 16:56

2 Answers2

1

The code you posted cannot possibly have the behavior described. The first call to Task.Start would throw an InvalidOperationException, failing the test.

I read that I can use Task.WhenAll() and Task.WaitAll() for precisely this scenario.

No; to asynchronously wait on multiple tasks, you must use Task.WhenAll, not Task.WaitAll.

Example:

[Fact]
public async Task GetLdapEntries_ReturnsLdapEntries()
{
  var ldapEntries = new List<int> { 0, 1 };
  var creationTasks = new List<Task>();
  foreach (var led in ldapEntries)
  {
    var task = CreateLdapEntry(led);
    creationTasks.Add(task);
  }
  await Task.WhenAll(creationTasks);
}

public async Task<string> CreateLdapEntry(int ldapEntryDto)
{
  await Task.Delay(500);
  return "";
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Thanks for the input, but it does not throw any exception, I've run the test multiple times, always hitting the deadlock. I'll try it with your example tomorrow. – Thaoden Feb 19 '19 at 19:00
  • As I tried to explain in the original question, `Task.WhenAll` as used as you propose is deadlocking – Thaoden Feb 20 '19 at 16:05
  • @Thaoden: I've updated the code. I've run this exact code without deadlock. – Stephen Cleary Feb 20 '19 at 17:29
  • I did in the meantime and found out the underlying error is in the webservice called by `_httpClient`. Thanks for your help! – Thaoden Feb 20 '19 at 18:43
0

Task.WaitAll() will deadlock simply because it blocks the current thread while the tasks are not finished (and since you are using async/await and not threads, all of your tasks are running on the same thread, and you are not letting your awaited tasks to go back to the calling point because the thread they are running in -the same one where you called Task.WaitAll()-, is blocked).

Not sure why WhenAll is also deadlocking for you here though, it definitely shouldn't.

PS: you don't need to call Start on tasks returned by an async method: they are "hot" (already started) already upon creation

Jcl
  • 27,696
  • 5
  • 61
  • 92