1

Can the following case be assumed de facto thread-safe? The bool flag always starts at false and only goes one way to true or stays as is. I know it's actually not thread-safe, but is it possible to really break somehow?

In this case the async execution is even strictly ordered and never parallel.

bool cancelled = false;

await MyAsyncJob.Process(() =>
{
    if (timeout)
    {
        cancelled = true;
    }
});

if (cancelled)
{
    DoSomething();
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Benjamin E.
  • 5,042
  • 5
  • 38
  • 65
  • 1
    You might want to mark the boolean as [`volatile`](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile), or alternatively use a `CancellationToken` and `CancellationTokenSource`. – ProgrammingLlama Feb 05 '20 at 07:45
  • You might? Should? Or even must? – Benjamin E. Feb 05 '20 at 07:46
  • Oh right, the cancellation source is thread-safe and accessible outside the loop! – Benjamin E. Feb 05 '20 at 07:47
  • 1
    I don't see any problem to this. Based on that the lambda is fully executed. – Jeroen van Langen Feb 05 '20 at 08:02
  • 1
    BTW, _["**I discourage you from ever making a volatile field**. Volatile fields are a sign that you are doing something downright crazy" - Eric Lippet](https://stackoverflow.com/a/17530556/585968)_. Ohad's emphasis. Originally from archived MSDN blog http://blogs.msdn.com/b/ericlippert/archive/2011/06/16/atomicity-volatility-and-immutability-are-different-part-three.aspx –  Feb 05 '20 at 08:02
  • 2
    Stephen Cleary has made some insightful comments about this issue: *It is not documented, but Microsoft people have assured me that the barriers are always present if await causes a thread switch.* [¹](https://stackoverflow.com/questions/59649998/does-net-resume-an-await-continuation-on-a-new-different-thread-pool-thread-or) *await injects all necessary thread barriers, so there's no issues around out-of-order reads or anything like that.* [²](https://stackoverflow.com/questions/59491873/do-i-need-to-synchronize-resource-access-between-tasks-with-the-default-tasksche/59503720#59503720) – Theodor Zoulias Feb 05 '20 at 08:33

1 Answers1

1

Can the following case be assumed de facto thread-safe? The bool flag...

It's not clear which flag you're referring to.

In your code, the cancelled flag will work as you expect, because it is checked after the method that sets it is awaited. The code running after the await will see all side-effects caused by the awaited method.

However, timeout does not look thread-safe at all. The awaited code checks timeout but is (probably) not awaiting whatever code sets timeout.

You could go through the trouble of using locks and whatnot, or you can use the CancellationTokenSource, CancellationToken, and OperationCanceledException provided by the .NET framework for exactly this scenario:

try
{
  await MyAsyncJob.Process(() =>
  {
    cancellationToken.ThrowIfCancellationRequested();
  });
}
catch (OperationCanceledException)
{
  DoSomething();
}

By following the standard pattern, other developers will be able to understand your code more quickly. And you won't have to worry about thread safety at all; the provided framework types take care of it.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Couldn't you also check `cancellationToken.IsCancellationRequested` after the await-block, avoiding throwing? This assumes of course cancellation is requested inside the await-block on timeout. – Benjamin E. Feb 06 '20 at 03:09
  • 1
    @BenjaminE. The standard pattern is to raise an `OperationCanceledException` exception when an operation is cancelled. You can choose to check `IsCancellationRequested`, but if you pass the `CancellationToken` to any *other* methods, then they will respond with an exception if cancelled (following the standard pattern), so you usually end up needing the exception handling anyway. – Stephen Cleary Feb 06 '20 at 14:07