Semaphore lock resulting in a deadlock

Question

public class SampleData
{
 private static readonly Semaphore pool = new Semaphore(0,1);

 public string Data => getFromFile();

 private static string getFromFile()
 {
   pool.WaitOne();
   var data = 
   File.ReadAllText("somefilepath");
    pool.Release();
    return data;
  }
}

In program.cs

var tasks = new List<Task<string>>();
for(int i=0; i<=5; i++)
{
   tasks.Add(Task.Run<string>(()=> 
    new SampleData().Data));
}

Task.WaitAll(tasks.ToArray());

When I run this, it never completes the tasks. Can any one tell me what's the issue here?

Thanks.

See also https://stackoverflow.com/questions/63920216/semaphore-blocks-though-it-isnt-full — Peter Duniho, Feb 05 '21 at 08:40

score 0 · Answer 1 · answered Feb 05 '21 at 04:54

If getFromFile() throw an exception, semaphore will never be released and Task.WaitAll will wait forever, you need to move pool.Release() to finally section. That might be the reason for indefinite waiting.

private static string getFromFile()
 {
   pool.WaitOne();
   try {
     var data = File.ReadAllText("somefilepath");
     return data;
   }
   finally {
     pool.Release();
   }
  }
}

score 0 · Answer 2 · edited Feb 06 '21 at 19:10

First of all

You should consider using SemaphoreSlim if this is not cross process.

Secondly

In regards to your initialization of Semaphore(0,1). If initialCount is less than maximumCount, the effect is the same as if the current thread had called WaitOne (maximumCount minus initialCount) times.

That's to say, when you call pool.WaitOne(); in your current configuration, you are going to block until someone calls release (straight off the bat).

E.g.:

private static readonly Semaphore pool = new Semaphore(0,1);

static void Main(string[] args)
{
   pool.WaitOne();
   Console.WriteLine("This will never get executed");
}

It's likely you want:

Semaphore(1,1)
// or
SemaphoreSlim(1,1)

Lastly

You must always wrap these synchronization primitives in a try finally, or you will eventually deadlock if an exception gets thrown before Release.

SemaphoreSlim

await pool.WaitAsync();
try
{
   // do something
}
finally
{
   pool.Release();
}

Or if you really need to use a Semaphore

pool.WaitOne();
try
{

  // do something
}
finally
{
   pool.Release();
}

Semaphore lock resulting in a deadlock

2 Answers2