0

In my code I assume that outerFlag will be hit after innerFlag but it actually runs like fire and forget (innerFlag is hit after outerFlag). When I use Thread.Sleep instead of Task.Delay flags are hit in correct order.

Here is the code:

[Test]
public async Task Test() {

    bool innerFlag = false;
    bool outerFlag = false;

    await Lock(async () => {
        await Task.Delay(2000);
        innerFlag = true;
    });

    outerFlag = true;

    Thread.Sleep(1000);
    Thread.Sleep(2500);
}

private Task Lock(Action action) {
    return Task.Run(action);
}

I also noticed that when I call Task.Delay and set innerFlag without a Lock method but by direct lambda, it works as expected.

Can somebody explain such behaviour?

Palle Due
  • 5,929
  • 4
  • 17
  • 32
Alex Zaitsev
  • 2,544
  • 5
  • 18
  • 27
  • 2
    That's because `Lock` takes a delegate, and in your case this delegate returns a task. But you don't await that task, so it's a fire and forget task. Basically you just blast by `await Lock(...)` and the 2 second delay inside the delegate will wait and then set innerFlag to true, but you've already ignore that task and just continued to execute. – Lasse V. Karlsen Jan 29 '20 at 13:16
  • 1
    If you change `Lock` to `private async Task Lock(Func func) { await func(); }`, it might work better. – Lasse V. Karlsen Jan 29 '20 at 13:16
  • Does this answer your question? [await Task.Delay() vs. Task.Delay().Wait()](https://stackoverflow.com/questions/26798845/await-task-delay-vs-task-delay-wait) – default locale Jan 29 '20 at 13:16
  • 4
    Well of course it runs like fire and forget -- you pass a `Task`-returning method to something eating an `Action`, meaning the task so produced is ignored. In essence, you're getting exactly what you asked for. (Note that if you want actual locking/exclusion, which this decidedly is not, you want to use something like `SemaphoreSlim` or [`AsyncLock`](https://github.com/StephenCleary/AsyncEx/wiki/AsyncLock) -- do not attempt to reinvent the wheel here with your own boolean flags, as that will almost certainly run into subtle bugs.) – Jeroen Mostert Jan 29 '20 at 13:17
  • @defaultlocale no, your question doesn't answer this one – Alex Zaitsev Jan 29 '20 at 13:46
  • @LasseV.Karlsen thanks, your solution worked! You can post the answer – Alex Zaitsev Jan 29 '20 at 13:48
  • I'm pretty sure this has already been answered here, so I'd rather leave it for someone to find the duplicate and close it. – Lasse V. Karlsen Jan 29 '20 at 13:55
  • @JeroenMostert Hi, I am actually writing a lock logic using `SemaphoreSlim` :) Just removed this from my example, but now after returning Func it works well – Alex Zaitsev Jan 29 '20 at 13:56

1 Answers1

1

Your Lock method doesn't understand async delegates, so the async delegate you are trying to pass as argument is treated as async void. Async voids are problematic in all sorts of ways and should be avoided, unless they are used for their intended purpose, as event handlers.

To ensure that your method understand async delegates you must create an overload that accepts a Func<Task> as argument:

private Task Lock(Func<Task> func)
{
    return Task.Run(func);
}

Notice that the func argument can be passed directly to Task.Run, because this method understands async delegates too. Not all built-in methods understand async delegates, with notable examples the Task.Factory.StartNew and Parallel.ForEach. You must be cautious every time you add the async modifier in a delegate. You must be sure that the called method understands async delegates, or else you may end up with async voids and the havoc they create.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104