Reputation: 11
So I have a program that does a lot of Asynchronous commands (web calls, etc) and functions that it calls in parallel. To stop the Main loop from going off the rails and quitting before everything is done, I created a class variable, static int die
, which counts the number of functions still in execution. Simplified/Naive task queuing, I suppose. Before invoking a function, I die++;
and at the end of each function, die--;
, with this sitting at the end of my Main() function:
while (die > 0) { }
Console.WriteLine("Writing Log File and Exiting");
} //End of Main()
And it works fine when running in Debug.
When I run it in Release, my program executes, but then 'freezes'. Pausing the execution in Visual Studio tells me the program is sitting at my while statement. The Immediate window tells me that die
has a value of 0, and that die > 0
is false.
Why is my loop not terminating?
Here is a somewhat-close example that isnt actually my program and may or may not show the problem because it's a race condition.
using Microsoft.Extensions.Configuration;
using System;
using System.IO;
using System.Net.Http;
namespace SOExample
{
static class Program
{
static IConfigurationRoot appsettings;
static int die;
static HttpClient client;
static void Main()
{
appsettings = new ConfigurationBuilder().SetBasePath(Directory.GetParent(AppContext.BaseDirectory).FullName).AddJsonFile("appsettings.json", true).Build();
client = new HttpClient();
die = 1;
Function1();
while (die > 0) { }
}
static async void Function1()
{
await client.GetAsync("http://google.com"); //Just to clear the async error;
//Do Func1 Stuff
if (appsettings.GetValue<bool>("doFunc2"))
{
die++;
Function2();
}
die--;
}
static async void Function2()
{
//Do Other Function Stuff
await client.GetAsync("http://google.com"); //Just to clear the async error;
die--;
}
}
}
Following the answer below from Basic below, I replaced all the die modifiers with Interlock expressions to make them atomic, but it did not break the condition. I have tried a couple of solutions that appear to work; Thread.Sleep(1000);
inside the loop body, and changing the while condition to being !Equals(die,0)
(presumably, to lock down the value and compare it atomically), which both seem to work, although clearly I need to rethink the design as a whole.
Upvotes: 1
Views: 110
Reputation: 26756
die++
and die--
are not thread safe.
Internally they're multi-step operations... read, increment/decrement, write.
If two threads do the same operation at the same time, they're likely to miscount.
Say die is on 5
. Threads 1 and 2 both try to execute die--
simultaneously.
die
as 5die
as 5die
to be 4die
to be 4 (again)Classic race condition.
That's far less likely to happen in DEBUG mode (but not impossible) as there's more checking of threads, and more overhead in general, meaning you're much less likely to have two threads execute the same command simultaneously.
You should be using Interlocked.Increment(ref die)
and Interlocked.Decrement(ref die)
Those will add/subtract 1 in a thread-safe manner.
Another potential issue...
Your die
variable isn't declalred volatile
.
This will definitely be a difference with DEBUG builds
Very simplistically, it indicates the variable may be changed by multiple threads and thus each should re-fetch the "real" value rather than relying on a local cache.
It also limits the optimisations that can be added by the compiler (in single-threaded operation, the compiler can re-order statements to optimise CPU efficiency, so long as they don't impact the result. This can be a lot harder to guess in multi-threaded apps and the compiler will often get it wrong).
It's a little more involved than that, but the upshot is you need to use it
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile
Upvotes: 4