A case where blindly using += in .NET C# would drive you crazy.
The main goal of this article is to demonstrate how a simple detail in our code could cause huge problems which are so hard to diagnose and troubleshoot.
As an example, we would use the += operator.
Have you ever imagined that blindly using the += operator could drive you crazy?
In this article, I would show you how this could happen and how to fix it.
Example Base Code
Before diving into the explanation, let’s start with writing some base code for the example we are going to use.
First, we would define an Employee class.
public class Employee
{
public decimal Salary { get; set; }
}As you can see it is just a simple class with only one defined property, Salary.
Now, let’s say that we have a RaiseCalculator class that would be used to calculate the raise for an Employee. The code would be as follows:
public class RaiseCalculator
{
private static readonly object _lock = new object();
public decimal Calculate(Employee employee)
{
lock(_lock)
{
return (employee.Salary <= 1000) ? 500 :1000;
}
}
}As you can see the code is simple. We have:
private static readonly object _lock field defined to be used for locking.
public decimal Calculate(Employee employee) method which applies the actual calculation.
That’s it, now we have the foundation code for our example. Let’s now proceed with the rest of the explanation.
Brain Teaser
Now, let me show you something interesting.
Let’s say that we want to start using this RaiseCalculator class to calculate the raise of an Employee.
Then, we can write some code like this:
public static async Task Run()
{
var calculator = new RaiseCalculator();
for (var i = 1; i <= 10; i++)
{
var employee = new Employee { Salary = 1000 };
var tasks = new Task[2];
tasks[0] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});
tasks[1] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});
await Task.WhenAll(tasks);
Console.WriteLine("Final: " + employee.Salary); // Expected 2500
}
}What we can notice here:
We are using one instance of the RaiseCalculator class.
Then we have a loop for 10 times. We are using this loop to see if the result we get would always be the same for multiple runs.
Inside the loop, we are defining a new Employee and we set its Salary to 1000.
Then, we create two tasks to be run in parallel.
The first task is increasing the employee's salary with the raise calculated by the calculator.
Also, the second task is doing the same.
Then we wait for both tasks to finish and finally print the final salary of the same employee.
What we should keep in mind is that, although the two tasks would run in parallel, the calculations should be applied in series as there is locking inside the RaiseCalculator class.
Let me quickly remind you. The code was as follows:
public decimal Calculate(Employee employee)
{
lock(_lock)
{
return (employee.Salary <= 1000) ? 500 :1000;
}
}Therefore, what we should expect is:
The task to set the lock first should see the input Salary as 1000, then the raise should be 500 and finally, the new Salary should be 1000 + 500 = 1500.
The task to come later should see the input Salary as 1500, then the raise should be 1000 and finally, the new Salary should be 1500 + 1000 = 2500.
This means that the message to appear in the console for each iteration should be “Final: 2500”. However, would it actually be so?
When we run the code, we would get something like this:
See, the result is not the same every time. Sometimes it is 1500 or 2500 or even 2000.
I can hear now screaming:
What the hell?!!!! How come this happens?
Let me tell you…
A Glimpse Under The Hood
First, let me tell you that this code which we used:
tasks[0] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});
tasks[1] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});Is actually equivalent to this code:
tasks[0] = Task.Run(() => {
employee.Salary = employee.Salary + calculator.Calculate(employee);
});
tasks[1] = Task.Run(() => {
employee.Salary = employee.Salary + calculator.Calculate(employee);
});That’s obvious, right?
However, what needs more attention is what is actually happening with the locking.
In the line of code employee.Salary + calculator.Calculate(employee); we start with capturing the current value of the employee.Salary. This is done outside the locking scope.
This means that while both tasks are racing, they could -it depends on when and how fast the tasks start- both capture the same starting value of the current Salary.
Therefore, even though there is locking inside the RaiseCalculator class, the initial starting value captured by each task might be wrong.
You can check the flow diagram above to fully understand what we are talking about.
Now you see it, right?
The Right Way
Now, let’s see how to fix this problem.
The fix is so simple, just use this code:
tasks[0] = Task.Run(() => {
employee.Salary = calculator.Calculate(employee) + employee.Salary;
});
tasks[1] = Task.Run(() => {
employee.Salary = calculator.Calculate(employee) + employee.Salary;
});Instead of this code:
tasks[0] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});
tasks[1] = Task.Run(() => {
employee.Salary += calculator.Calculate(employee);
});This way, the flow will be as follows:
This way, capturing the current value of the employee.Salary comes after releasing the lock. Therefore, the first task would have enough time to capture this value and add it to the employee.Salary before the second task proceeds with the calculation.
So, if we run the new code, we would get this result:
I can hear you now saying:
Still this is not good enough. What if the second task was so fast that it was able to acquire the current salary even before the first task adds to it?
Final Thoughts
I totally agree with you. That’s why if I were going to design this module I would never do it this way.
However, sometimes we can’t redesign some modules because we don’t own them or for other reasons.
Therefore, the main goal of this example is to show you how a small piece of code could cause a lot of trouble.
This is why we need to deal with every line of code as an essential part of the whole system.
That’s it. Hope you find reading this article as interesting as I found writing it.
Informational Content is the food for mind. I really appreciate it.😀