Async semaphore with priority: Rethinking queuing continuations
When I wrote the AsyncPrioritySemaphore few months ago I quickly found how the continuations are processed once “unleashed”. Lately I was using the same class again and I was monitoring the behavior. That made me think about what could be done and how.
Let me quickly recap. The original Release method looked like this.
public void Release()
{
TaskCompletionSource<object> toRelease = null;
lock (_syncRoot)
{
if (_waitersHigh.Count > 0)
toRelease = _waitersHigh.Dequeue();
else if (_waitersNormal.Count > 0)
toRelease = _waitersNormal.Dequeue();
else
++_currentCount;
}
if (toRelease != null)
{
// separate task to avoid stack overflow on continuations
Task.Factory.StartNew(o => (o as TaskCompletionSource<object>).SetResult(null), toRelease, TaskCreationOptions.HideScheduler).Wait();
}
}
As the comment says I’m calling the SetResult method using a Task to avoid stack overflows. The SetResult call the continuations synchronously, hence if you have a deep chain of methods waiting on the semaphore you’ll run out of stack.
I’m also calling Wait on the Task. My original reasoning was that I want to be sure the continuations are processed and I, if any, receive (don’t loose) exceptions (although I’m not handling exception, I just want them to bubble up). There was also a discussion about this in comments. Read it if you’re interested.
Recently I was improving my knowledge about async/await internals and threading/locking internals and decided to re-evaluate my original reasoning. After some testing I decided to change the method.
public void Release()
{
TaskCompletionSource<object> toRelease = null;
lock (_syncRoot)
{
if (_waitersHigh.Count > 0)
toRelease = _waitersHigh.Dequeue();
else if (_waitersNormal.Count > 0)
toRelease = _waitersNormal.Dequeue();
else
++_currentCount;
}
if (toRelease != null)
{
// break the stack to avoid stack overflow on continuations
ThreadPool.QueueUserWorkItem(o => (o as TaskCompletionSource<object>).SetResult(null), toRelease);
}
}
I ditched waiting and also used ThreadPool class directly. Why no longer the waiting? As you release the lock you want others to continue running (if there are resources) and yourself as well. Releasing the semaphore should just open the gate(s) and off you go. Waiting for others to complete doesn’t make sense. Also as I learned how the exceptions from continuations are be handled, I realized the Wait is not needed. I don’t loose them.
Once the Wait was gone there was little reason to keep using Task. I could queue the method to ThreadPool directly myself (if you don’t say otherwise the Task uses ThreadPool). I was also thinking about using UnsafeQueueUserWorkItem method, but I can’t prove (or disprove) myself that it’s correct (or not). So I just took the safe path. Maybe in next iteration. 😃
I’m running it in, same as previous version, a high-load environment. Hope I’ll learn something new again.
