Reentrance method and partial synchronized calls

Viewed 29

I do have a singleton component that manages some information blocks. An information block is a calculated information identified by some characteristics (concrete an Id and a time period). These calculations may take some seconds. All information blocks are stored in a collection.

Some other consumers are using these information blocks. The calculation should start when the first request for this Id and time period comes. I had following flow in mind:

  1. The first consumer requests the data identified by Id and time period.
  2. The component checks if the information block already exists
  3. If not: Create the information block, put it into the collection and start the calculation in a background task. If yes: Take it from the collection

After that the flow goes to the information block:

  1. When the calculation is already finished (by a former call), a callback from the consumer is called with the result of the calculation.
  2. When the calculation is still in process, the callback is called when the calculation is finished.

So long, so good.

The critical section comes when the second (or any other subsequent) call is coming and the calculation is still running. The idea is that the calculation method holds each consumers callback and then when the calculation is finished all consumers callbacks are called.

public class SingletonInformationService
{
    private readonly Collection<InformationBlock> blocks = new();
    private object syncObject = new();

    public void GetInformationBlock(Guid id, TimePersiod timePeriod,
        Action<InformationBlock> callOnFinish)
    {
        InformationBlock block = null;
        
        lock(syncObject)
        {
            // check out if the block already exists
            block = blocks.SingleOrDefault(b => b.Id ...);
            
            if (block == null)
            {
                block = new InformationBlock(...);
                blocks.Add(block);
            }
        }
        
        block?.BeginCalculation(callOnFinish);
        
        return true;
    }
} 

public class InformationBlock
{
    private Task calculationTask = null;
    private CalculationState isCalculating isCalculating = CalculationState.Unknown;
    private List<Action<InformationBlock> waitingRoom = new();
    
    internal void BeginCalculation(Action<InformationBlock> callOnFinish)
    {
            if (isCalculating == CalculationState.Finished)
            {
                callOnFinish(this);
                return;
            }
            else if (isCalculating == CalculationState.IsRunning)
            {
                waitingRoom.Add(callOnFinish);
                return;
            }

            // add the first call to the waitingRoom
            waitingRoom.Add(callOnFinish);
                
            isCalculating = CalculationState.IsRunning;
            
            calculationTask = Task.Run(() => { // run the calculation})
                    .ContinueWith(taskResult => 
                    {
                        //.. apply the calculation result to local properties
                        this.Property1 = taskResult.Result.Property1;
                        
                        // set the state to mark this instance as complete
                        isCalculating = CalculationState.Finished;
                                                
                        // inform all calls about the result
                        waitingRoom.ForEach(c => c(this));
                                                
                        waitingRoom.Clear();
                    }, TaskScheduler.FromCurrentSynchronizationContext());
    }
}

Is that approach a good idea? Do you see any failures or possible deadlocks? The method BeginCalculation might be called more than once while the calculation is running. Should I await for the calculationTask?

1 Answers

To have deadlocks, you'll need some cycles: object A depends of object B, that depends on object A again (image below). As I see, that's not your case, since the InformationBlock class doesn't access the service, but is only called by it. The lock block is also very small, so probably it'll not put you in troubles.

You could look for the Thread-Safe Collection from C# standard libs. This could simplify your code. I suggest you to use a ConcurrentDictionary, because it's fastest then iterate over the collection every request.

Deadlock example (random image from Google)

Related