I have an Asp.Net Core 6 Web Api.

I have a Singleton class with several methods in it.

I want only 1 thread to enter any of the methods of the class at a time.

Is it ok to initialize the SemaphoreSlim class in the constructor and use it in every method in the following way? Are there any dangers from it?

Is there a better way to achieve what I am looking for?

public class Foo
{
   private readonly SemaphoreSlim _sm;

   public Foo()
   { 
      _sm = new SemaphoreSlim(1, 1);
   }

   public async Task FirstMethod()
   {
      await _sm.WaitAsync();

      //Do some work
     
      _sm.Release();
   }

   public async Task SecondMethod()
   {
      await _sm.WaitAsync();

      //Do some work
     
      _sm.Release();
   }
}

CodePudding user response：

You should adopt the try/catch/finalize pattern - if you do so, you're mostly safe: all code under the WaitAsync will be executed as defined by the max semaphore. Only if the code under the semaphore block, you'll get a deadlock - you could consider using a timeout, but typically this is accepted.

   public async Task SecondMethod()
   {
      await _sm.WaitAsync();

      try
      {
          //Do some work
      }
      
      finally
      {
         //release in case of errors
         _sm.Release();
      }
   }

Other stuff to consider is, especially if it is a long running process, is to use a cancellation token to indicate application closure.

Also keep in mind that if you fire a lot of these methods from various threads, the order is not guaranteed - but they will all be handled.