mercredi 3 novembre 2021

Potential race conditions with ConcurrentBag and multithreaded application

I've been wrestling for the past few months with how to improve a process where I'm using a DispatcherTimer to periodically check resources to see if they need to be updated/processed. After updating the resource("Product"), move the Product to the next step in the process, etc. The resource may or may not be available immediately.

The reason I have been struggling is two-fold. One reason is that I want to implement this process asynchronously, since it is just synchronous at the moment. The second reason is that I have identified the area where my implementation is stuck and it seems like not an uncommon design pattern but I have no idea how to describe it succinctly, so I can't figure out how to get a useful answer from google.

A rather important note is that I am accessing these Products via direct USB connection, so I am using LibUsbDotNet to interface with the devices. I have made the USB connections asyncronous so I can connect to multiple Products at the same time and process an arbitrary number at once.


public Class Product
{
 public bool IsSoftwareUpdated = false;
 public bool IsProductInformationCorrect = false;
 public bool IsEOLProcessingCompleted = false;

 public Product(){}
 ~Product()
}

public class ProcessProduct
{
 List<Product> bagOfProducts                   = new List<Product>(new Product[10]);

 ConcurrentBag<Product> UnprocessedUnits       = new ConcurrentBag<Product>();
 ConcurrentBag<Product> CurrentlyUpdating      = new ConcurrentBag<Product>();
 ConcurrentBag<Product> CurrentlyVerifyingInfo = new ConcurrentBag<Product>();
 ConcurrentBag<Product> FinishedProcessing     = new ConcurrentBag<Product>();

 DispatcherTimer _timer = new DispatcherTimer();

 public ProcessProduct()
 {
     _timer.Tick += Timer_Tick;                            //Every 1 second, call Timer_Tick
     _timer.Interval = new TimeSpan(0,0,1);                //1 Second timer
     
     bagOfProducts.ForEach(o => UnprocessedUnits.Add(o));  //Fill the UnprocessedUnits with all products
 
     StartProcessing();
 }
 private void StartProcessing()
 {
     _timer.Start();
 }

 private void Timer_Tick(object sender, EventArgs e)
 {
     ProductOrganizationHandler();

     foreach(Product prod in CurrentlyUpdating.ToList())
     {
         UpdateProcessHandler(prod);  //Async function that uses await
     }
     foreach(Product prod in CurrentlyVerifyingInfo.ToList())
     {
         VerifyingInfoHandler(prod);  //Async function that uses Await
     }
     if(FinishedProcessing.Count == bagOfProducts.Count)
     {
         _timer.Stop();  //If all items have finished processing, then stop the process
     }
 }
 
 private void ProductOrganizationHandler()
 {
     //Take(read REMOVE) Product from each ConcurrentBag  1 by 1 and moves that item to the bag that it needs to go
     //depending on which process step is finished
     //(or puts it back in the same bag if that step was not finished).
     //E.G, all items are moved from UnprocessUnits to CurrentlyUpdating or CurrentlyVerifying etc.
     //If a product is finished updating, it is moved from CurrentlyUpdating to CurrentlyVerifying or FinishedProcessing
 }
 private async void UpdateProcessHandler(Product prod)
 {
     await Task.Delay(1000).ConfigureAwait(false);
     //Does some actual work validating USB communication and then running through the USB update
 }
 private async void VerifyingInfoHandler(Product prod)
 {
     await Task.Delay(1000).ConfigureAwait(false);
     //Does actual work here and communicates with the product via USB
 }
}

Full Compile-ready code example available via my code on Pastebin.

So, my question really is this: Are there any meaningful race conditions in this code? Specifically, with the ProductOrganizationHandler() code and the looping through the ConcurrentBags in Timer_Tick() (since a new call to Timer_Tick() happens every second). I'm sure this code works the majority of the time, but I am afraid of a hard-to-track bug later on that happens because of a rare race condition when, say, ProductOrganizationHandler() takes > 1 sec to run for some dumb reason.

As a secondary note: Is this even the best design pattern for this type of process? C# is my first OOP language and all self-taught on the job (nearly all of my job is Embedded C) so I don't have any formal experience with OOP design patterns.

My main goal is to asynchronously Update/Verify/Communicate with each device as it becomes available via USB. Once all products in the list are finished (or a timeout), then the process finishes. This project is in .NET 5.

Aucun commentaire:

Enregistrer un commentaire