Is it okay for async function to update a common object

Question

I have a couple of functions, each function verifies a set of rules and updates a common object. The common object is just a container that holds a list of rules that passed or failed. I would like to implement this as async. Is there any downside to this approach? Are there any recommendations? The sample code I intend to implement is given below.

using System.Collections.Generic;
using System.Threading.Tasks;
namespace RetailServiceCheck.Core
{
 //This is shared result
 public class Results
 {
 public List<int> FailedRules = new List<int>();
 public List<int> PassedRules = new List<int>();
 }
 //To Check the rules
 public class RuleChecker
 {
 Results ruleResult = null;
 public RuleChecker()
 {
 ruleResult=new Results();
 }
 public Results ValidateRule()
 {
 var ckr1=CheckRule1();
 var ckr2=CheckRule2();
 var ckr3=CheckRule3();
 Task.WaitAll(ckr1, ckr2, ckr3);
 return ruleResult;
 }
 private async Task CheckRule1()
 {
 //Do some rule check and if the based on rule failure or pass
 ruleResult.PassedRules.Add(1);
 }
 private async Task CheckRule2()
 {
 //Do some rule check and if the based on rule failure or pass
 ruleResult.FailedRules.Add(2);
 }
 private async Task CheckRule3()
 {
 //Do some rule check and if the based on rule failure or pass
 ruleResult.PassedRules.Add(3);
 }
 }
 }

• David Arno: "I would like to implement this as async". Why?
• BipinR: @DavidArno Each of the rules has a bit of complicated rule and takes some time to complete, my thought was if I implement this as async it could improve the performance.
• David Arno: That's possible. But if the tasks can be run in parallel, then you could run into thread safety issues. And if they can be suspended whilst another action occurs in a synchronous fashion, you could end up with the results being written in a non-deterministic order (maybe not a problem). So I'd recommend each return its result and then, when they all complete, then you update ruleResult to avoid such issues.

Answer 1

Having each async method update a common object risks running into threading/non-deterministic order issues with the results. So rather than have each async method update a common object, have each return a result, then update the results when all have completed:

 public Results ValidateRule()
 {
 var ckr1=CheckRule1();
 var ckr2=CheckRule2();
 var ckr3=CheckRule3();
 Task.WaitAll(ckr1, ckr2, ckr3);
 UpdateRuleResult(ckr1.Result);
 UpdateRuleResult(ckr2.Result);
 UpdateRuleResult(ckr3.Result);
 }
 private void UpdateRuleResult((bool passed, int value))
 {
 if (passed)
 {
 ruleResult.PassedRules.Add(value);
 }
 else
 {
 ruleResult.FailedRules.Add(value);
 }
 }
 private async Task<(bool passed, int value)> CheckRule1()
 {
 //Do some rule check and if the based on rule failure or pass
 return (true, 1);
 }
 etc

Answer 2

A big concern with asynchronous functions is state management. If your function requires data that might be changed by an asynchronous function, then its possible for your data to change or become stale before or mid execution, causing unexpected results that can be difficult to debug.

If you can set up your functions so that they will take data that will not be changed as inputs and have each save to a different set of unique fields (a different rule in your case), then you might be able to avoid this side effect with asynchronous code and make it work.

If not, then you have to set up things like locking to make this all work and you might still run into state bugs if not done properly.

• BipinR: In the above sample, the data used by the aync function for processing the rules are not changing, only the Results object is changed by each of the async functions. Do you mean that will be a problem? The order in which the data is added to the list of passed or failed does not matter.
• Nathanael: @BipinR: Probably not. However, say if the functions ever need to check the Results object at some point, that would be a problem because suddenly the order does matter. But as long as these things are true it could be a good candidate for using async.