.NET 6 Async Semaphore Error Under Mild Load

Question

I'm working on a basic (non DB) connection pool which allows only 1 connection to be created per project.

The connection pool supports an async-task/threaded environment and therefor I have made use of a semaphore instead of a regular Lock.

I wrote a test, below, which is meant to stress test the connection pool. The code works but under higher loads, the semaphore throws the following error

I can overcome this error by decreasing the load.

For example, increasing the _waitTimeMs to a higher number (i.e. 50ms or 100ms or 1000ms) or decreasing _numberOfTasks (i.e. to 5 or 3). I should also mention that sometimes, it manages to run higher load tests without errors.

Is there a mistake or misconception in my code and/or use of semaphores?

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

internal class Program
{
    static int _numberOfTasks = 50;
    static int _waitTimeMs = 1;
    static SemaphoreSlim _dictLock = new SemaphoreSlim(1, 1);
    static ConcurrentDictionary<string, bool> _pool = new ConcurrentDictionary<string, bool>();

    /// <summary>
    /// Only 1 connection allowed per project.
    /// We reuse connections if available in pool, otherwise we create 1 new connection.
    /// </summary>
    static async Task<string> GetConnection(string projId)
    {
        try
        {
            // Enter sema lock to prevent more than 1 connection
            // from being added for the same project.
            if (await _dictLock.WaitAsync(_waitTimeMs))
            {
                // Try retrieve connection from pool
                if (_pool.TryGetValue(projId, out bool value))
                {
                    if (value == false)
                        return "Exists but not connected yet.";
                    else
                        return "Success, exists and connected.";
                }
                // Else add connection to pool
                else
                {
                    _pool.TryAdd(projId, false);

                    // Simulate delay in establishing new connection
                    await Task.Delay(2);

                    _pool.TryUpdate(projId, true, false);

                    return "Created new connection successfully & added to pool.";
                }
            }
            // Report failure to acquire lock in time.
            else
                return "Server busy. Please try again later.";
        }
        catch (Exception ex)
        {
            return "Error " + ex.Message;
        }
        finally
        {
            // Ensure our lock is released.
            _dictLock.Release();
        }
    }

    static async Task Main(string[] args)
    {
        if (true)
        {
            // Create a collection of the same tasks
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < _numberOfTasks; i++)
            {
                // Each task will try to get an existing or create new connection to Project1
                var t = new Task(async () => { Console.WriteLine(await GetConnection("Project1")); });
                tasks.Add(t);
            }

            // Execute these tasks in parallel.
            Parallel.ForEach<Task>(tasks, (t) => { t.Start(); });
            Task.WaitAll(tasks.ToArray());
            Console.WriteLine("Done");
            Console.Read();
        }
    }
}

Answer 1

Is there a mistake or misconception in my code and/or use of semaphores?

There's a bug in your code, yes. If the WaitAsync returns false (indicating that the semaphore was not taken), then the semaphore is still released in the finally block.

If you must use a timeout with WaitAsync (which is highly unusual and questionable), then your code should only call Release if the semaphore was actually taken.