I am using a static variables to get access between threads, but is taking so long to get their values.
Context: I have a static class Results.cs
, where I store the result variables of two running Process.cs
instances.
public static int ResultsStation0 { get; set; }
public static int ResultsStation1 { get; set; }
Then, a function of the two process instances is called at the same time, with initial value of ResultsStation0/1 = -1.
Because the result will be provided not at the same time, the function is checking that both results are available. The fast instance will set the result and await for the result of the slower instance.
void StationResult(){
Stopwatch sw = new Stopwatch();
sw.Restart();
switch (stationIndex) //Set the result of the station thread
{
case 0: Results.ResultsStation0 = 1; break;
case 1: Results.ResultsStation1 = 1; break;
}
//Waits to get the results of both threads
while (true)
{
if (Results.ResultsStation0 != -1 && Results.ResultsStation1 != -1)
{
break;
}
}
Trace_Info("GOT RESULTS " stationIndex "Time: " sw.ElapsedMilliseconds.ToString() "ms");
if (Results.ResultsStation0 == 1 && Results.ResultsStation1 == 1)
{
//set OK if both results are OK
Device.profinet.WritePorts(new Enum[] { NOK, OK },
new int[] { 0, 1 });
}
}
It works, but the problem is that the value of sw of the thread that awaits, should be 1ms more or less. I am getting 1ms sometimes, but most of the times I have values up to 80ms. My question is: why it takes that much if they are sharing the same memory (I guess)?
Is this the right way to access to a variable between threads?
CodePudding user response:
Don't use this method. Global mutable state is bad enough. Mixing in multiple threads sounds like a recipe for unmaintainable code. Since there is no synchronization at all in sight there is no real guarantee that your program may ever finish. On a single CPU system your loop will prevent any real work from actually being done until the scheduler picks one of the worker threads to run, an even on multi core system you will waste a ton of CPU cycles.
If you really want global variables, these should be something that can signal the completion of the operation, i.e. a Task, or ManualResetEvent. That way you can get rid of your horrible spin-wait, and actually wait for each task to complete.
But I would highly recommend to get rid of the global variables and just use standard task based programming:
var result1 = Task.Run(MyMethod1);
var result2 = Task.Run(MyMethod2);
await Task.WhenAll(new []{result1, result2});
Such code is much easier to reason about and understand.
Multi threaded programming is difficult. There are a bunch of new ways your program can break, and the compiler will not help you. You are lucky if you even get an exception, in many cases you will just get an incorrect result. If you are unlucky you will only get incorrect results in production, not in development or testing. So you should read a fair amount about the topic so that you are at least familiar with the common dangers and the ways to mitigate them.
CodePudding user response:
You are using flags as signaling for this you have a class called AutoResetEvent
.
There's a difference between safe access
and synchronization
.
- For safe access (atomic) purpose you can use the class
Interlocked
- For synchronization you use
mutex
based solutions - either spinlocks, barriers, etc...
What it looks like is you need a synchronization mechanism because you relay on an atomic
behavior to signal a process that it is done.
Further more,
For C#
there's the async
way to do things and that is to call await
.
It is Task
based so in case you can redesign your flow to use Task
s instead of Threads it will suit you more.
Just to be clear - atomicity means you perform the call in one go. So for example this is not atomic
int a = 0;
int b = a; //not atomic - read 'a' and then assign to 'b'.
I won't teach you everything to know about threading in C# in one post answer - so my advice is to read the MSDN articles about threading and tasks.