HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Async Queue Processing

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asyncprocessingqueue

Problem

I have a Queue that needs to try to process when anything updates in it (add, remove or update).

However, I don't want any of the callers to wait while the processing is happening (either for the processing to happen or while the processing is happening).

This is what I came up with:

private static readonly SemaphoreSlim asyncLock = new SemaphoreSlim(1);
private async void ProcessQueue()
{
    // Lock this up so that one thread at a time can get through here.  
    // Others will do an async await until it is their turn.
    await asyncLock.WaitAsync();
    try
    {
        // Offload this to a background thread (so that the UI is not affected)
        var queueProcessingTask = Task.Run( () =>
        {
            var processingStuck = false;
            while (myQueue.Count >= 1 && !processingStuck)
            {
                // Get the next item
                var queueItem = myQueue.Peek();
                // Try to process this one. (ie DoStuff)
                processingStuck = ProcessQueueItem(queueItem);
                // If we processed successfully, then we can dequeue the item
                if (!processingStuck)
                    myQueue.Dequeue();
            }                
        });
        Task.WaitAll(queueProcessingTask);
    }
    finally
    {
        asyncLock.Release();
    }    
}


Is my Thread and async handling going to ensure that only 1 queueItem at a time can ever be "in the works"?

And will this avoid using resources from the UI thread?

Solution

The thread that calls it will likely be the UI Thread.

You're invoking Task.WaitAll which waits for the processing to finish.

However you said, "... I don't want any of the callers to wait while the processing is happening (either for the processing to happen or while the processing is happening)."

Is this a contradiction?

Instead of creating short-lived tasks as a local variable inside a semaphore, perhaps it would be better to:

  • Create a single long-lived task (e.g. as a data member of your class)



  • Wake up the task when you enqueue something or fix a ProcessQueueItems problem (perhaps by setting a event which the task is waiting on)



  • Don't invoke Task.WaitAll until your program is shutting down



  • Put an exception handler inside the task so that the (single, supposedly long-lived) task can't permanently fail if one ProcessQueueItem call throws an exception



It is a custom type of mine. Has a few native types and a dictionary.

It needs to be a type like ConcurrentQueue because you have one thread (the task) dequeueing from it while (presumably) other threads are enqueueing on it.

Context

StackExchange Code Review Q#42893, answer score: 5

Revisions (0)

No revisions yet.