0

I have a class with quite a lot of public functions. I want to prevent the situation that any of the function is executed when another function is already running. So I included a semaphore in every function like this:

public bool FunctionX(int a)
{
    if (semaphore.Wait(0))
    {
        try
        {
            return DoSomething(a);
        }
        finally
        {
            semaphore.Release();
        }
    }
    else
        return false;
}

This works, but is there a more elegant way, where I do not have to write this wrapping semaphore stuff each time? I am thinking about something of the form

public bool FunctionX(int a)
{
    WrapSemaphoreAround(DoSomething(a))
}

Note that the signature of every function is different, not necessarily bool as return value and int as parameter, so it also has to work with

public string FunctionY(byte a, bool b)
{
    WrapSemaphoreAround(DoAnotherThing(a, b))
}

Is this even possible in C#?

Zoe
  • 27,060
  • 21
  • 118
  • 148
Thern
  • 1,017
  • 8
  • 17
  • 1
    Maybe this helps: [What is Func, how and when is it used](https://stackoverflow.com/questions/3624731/what-is-func-how-and-when-is-it-used) – Hans Kesting Sep 14 '22 at 06:32
  • 1
    Unless this is a homework question, it might be better to look at why everything needs to be locked, and if you can use a work queue or similar instead. – Iain Ballard Sep 14 '22 at 06:36
  • @IainBallard There is a GUI around that calls the different functions, which all belong to a Bluetooth Module. I do not want to send more than one Bluetooth command at a time to avoid problems with the Device Manager. – Thern Sep 14 '22 at 06:51
  • Then a queue of messages/jobs/tasks sounds an ideal fit, especially once you consider timeouts and failures. – Iain Ballard Sep 14 '22 at 06:54
  • I am not sure if this is the best fit. It would mean piling up the requests instead of discarding them if e.g. the user clicked on them several times. For instance, if the user clicked the Scan button several times, I do not want to run the Scan procedure several times, but inform them that a procedure is already running. – Thern Sep 14 '22 at 07:03
  • Are the functions _writing_ data to the class? Is that why you want to block parallelism? Have you tried using `lock(this) { .. }` instead? – JAlex Sep 14 '22 at 17:46
  • @Thern - can you just disable (gray out) the buttons while processing. This would be the best option as the user would know what is going on. – JAlex Sep 14 '22 at 17:47
  • GUIs are typically single-threaded and driven by an event queue. The normal way to handle this would be to disable or hide some part of the UI before starting the background process, then re-enable it when the process completes. There's no need for low level concurrency stuff like semaphores. – Kevin Krumwiede Sep 14 '22 at 17:51

1 Answers1

3

If you want to surround a method with something else you can use a delegate:

public bool DoX() => DoPreparationAndCleanup(DoXImpl);
private bool DoXImpl(){
 ...
}
private T DoPreparationAndCleanup<T>(Func<T> a){
    // do preparation
    var result = a();
    // do cleanup
    return result;
}

However, using a semaphore in that way is probably not a great idea. If some method is called by multiple threads, one or the other will fail, and what are they supposed to do in that case? Try again some time later? Just fail? In c#, a semaphore is a specialized thread safety primitive, and should be reserved for specialized use cases.

If you want to block a UI from doing anything else while the operation is in progress, my go to method is a Modal Dialog, i.e. using wpf:

var myDialog = new MyWindow();
var task = Task.Run(MySlowMethod);
task.ContinueWith( t => Dispatcher.BeginInvoke((Action)myDialog.Close));
myDialog.ShowDialog();

This will run MySlowMethod on a background thread, when this is done it will send a message to the UI thread to close the dialog. While it is running only the controls inside MyWindow can be used.

The standard primitive to ensure thread safety is a lock, that will block execution in a critical section to ensure only a single thread hold the lock at any one time. All other threads will block until the lock becomes available. This should give a more predicable program behavior, but do introduce the possibility of deadlocks.

private object myLockObject = new object();
public bool DoX(int a)
{
    lock(myLockObject){
       return DoXImpl(a);
    }
}

Ofc, even better than locks is ensuring your methods are thread safe by default, for example by using immutability and pure functions.

Another way to ensure exclusive access to some resource is with a limitedconcurrencyTaskSceduler (see example). This allow you to start multiple tasks, where only one is allowed to run at any one time.

JonasH
  • 28,608
  • 2
  • 10
  • 23
  • A lock is not a good option. If a function is called and another function is already in process, I want the function not to be executed and returning with the info that the module is currently executing an operation. If using a lock, the second method would instead wait to be executed, so I would pile up functions waiting for execution. And I can't inherently make the functions thread safe because they involve the Windows Device Manager for Bluetooth and this sometimes makes strange things when called again without finishing the first call. – Thern Sep 14 '22 at 06:57
  • @Thern Then it does not sound like you actually want any concurrency support. If you are writing a UI program such things are much better handled by for example showing a modal window, preventing any other operations from starting, or by Disabling/enabling buttons or controls, or by storing a task that can be checked for completion. This is really starting to sound like an [XY-problem](https://xyproblem.info/). – JonasH Sep 14 '22 at 07:03
  • My GUI is just a test GUI. I am writing the BLE module, but the actual GUI will look very different, so I wanted to include inherent safety into the module instead of having the GUI handle that job. If there is a better way to achieve this, I am open for ideas, as my programming skills are limited and I could not think of a better idea than the semaphores. I don't even know if it is best practice to include these checks in the module or leave them to the GUI. – Thern Sep 14 '22 at 07:08
  • @Thern leave it to the UI. Either design your class to be safely used from multiple threads, or allow it to fail if used incorrectly. Unless otherwise specified, non-static methods should be assumed to be *non* thread safe. You could possibly set/reset a bool at entry/exit and a Debug.Assert to help you find programming errors faster. – JonasH Sep 14 '22 at 07:50
  • I see your point. In any case, you answered the question as it was asked, so I will mark your answer as correct. – Thern Sep 16 '22 at 11:37