Understanding SemaphoreSlim problems?

Viewed 3701

I have a simple test code here with SemaphoreSlim

        static SemaphoreSlim mSemaphore = new SemaphoreSlim(3);

        static async Task Main()
        {
            var tasks = new Task[5];

            for (var i = 0; i < tasks.Length; i++)
            {
                var taskNo = i;
                tasks[i] = Task.Run(() => DoWork($"task{taskNo}"));
            }

            await Task.WhenAll(tasks);
        }

        static async Task DoWork(string taskName)
        {
            for (var i = 0; i < 3; i++)
            {
                mSemaphore.Wait();

                Console.WriteLine($"{taskName}: doing {i}.");   
                await Task.Delay(1000); 

                mSemaphore.Release();
            }
        }

if i am correct: in my context my Semaphore should allow only 3 Tasks to do their work, and then it should release them, and after that it should let my other 2 Tasks to do their job.

The Problem

i tested this out but unfortunately sometimes i get different/wrong results.

here are 2 output results.

OUTPUT

enter image description here

2 Answers

The calls to mSemaphore.Wait() and mSemaphore.Release() are inside your for loop. After every iteration of the loop, each task releases the semaphore, then attempts to acquire it again.

Given this, there's nothing to stop task 0, 1, or 2 from releasing the semaphore at the end of a loop, and task3 from acquiring it. The task that released it will go back to the start of its loop, and sit on mSemaphore.Wait() again, waiting for another task to release the semaphore.

All of your tasks are running at the same time (apart from a very small initial delay, potentially), and they all have equal priority (SemaphoreSlim doesn't guarantee the order in which things waiting for the semaphore acquire it -- it's essentially random which waiting task will be given it). Therefore it's no surprise that sometimes tasks 0, 1 and 2 get to complete before task3 is given the semaphore, and sometimes things happen in a different order.

If you add some extra logging at the point that each task attempts to acquire the semaphore, actually acquires it, then releases it, that should make it a bit clearer -- you'll be able to see one task release it, and then another task immediately pick it up).

Method SemaphoreSlim.Wait:

Blocks the current thread until it can enter the SemaphoreSlim.

Method SemaphoreSlim.WaitAsync:

Asynchronously waits to enter the SemaphoreSlim.

Since your code is asynchronous, you should use WaitAsync, not Wait. By blocking threads while running asynchronous code, you'll get all kinds of funny effects instead of the behavior you expect. Consider for example that after await Task.Delay(1000) your asynchronous workflow may continue running on a different thread, or may not, depending on conditions that you don't control.

Long story short, just replace mSemaphore.Wait() with await mSemaphore.WaitAsync() and you'll be OK.

Related