-1

Given a scenario where there's a function that should only be executed by one thread at any given time, and the rest just return (since a specific state is already being worked on), what's the best way to accomplish this?

public void RunOnce() { if(Interlocked.Exchange(ref m_isRunning, 1) == 1) return; // Run code that should only be executed once // What mechanism do we use here to ensure thread safety? Volatile.Write(ref m_isRunning, 0); }

Would the same mechanism apply if m_isRunning is a state (ie. an integer representing an enum)?

Improve this question
11
  • Why are you trying to use a lockless implementation? Have you measured a performance problem with a solution using a proper synchronization primitive? Commented Aug 2, 2021 at 16:41
  • 1
    Writing lockless multi-threaded code is hard and hard to test (and fragile; maintenance programmers are known to occasionally delete essential (but seemingly useless) code from a working lockless implementation). C#'s lock mechanism is lightweight. Are you sure you need to roll-your-own? Commented Aug 2, 2021 at 16:45
  • @Flydog57 A contending lock isn't lightweight at all. Also, if we just keep telling people to use "lock" because the alternatives are difficult, no-one is ever going to learn how to write performant multi-threaded applications. Commented Aug 2, 2021 at 17:02
  • @Dennis19901 But you're returning when the lock is taken out, which will be real fast. Again, have you actually measured the performance of this and ensured it's unacceptable for your uses? Odds are it'll be fine. And odds that you code a lockless version that actually behaves properly in all situations are much lower. Correctness before performance. Commented Aug 2, 2021 at 17:07
  • 2
    Instead of Volatile.Write use Interlocked.Exchange(ref m_isRunning, 0); Commented Aug 2, 2021 at 18:01

1 Answer 1

Reset to default
1

The code in your question is thread-safe IMHO, but in general the Interlocked.CompareExchange method is more flexible than the Interlocked.Exchange for implementing lock-free multithreading. Here is how I would prefer to code the RunOnce method:

int _lock; // 0: not acquired, 1: acquired public void RunOnce() { bool lockTaken = Interlocked.CompareExchange(ref _lock, 1, 0) == 0; if (!lockTaken) return; try { // Run code that should be executed by one thread only. } finally { bool lockReleased = Interlocked.CompareExchange(ref _lock, 0, 1) == 1; if (!lockReleased) throw new InvalidOperationException("Could not release the lock."); } }

My suggestion though would be to use the Monitor class:

object _locker = new(); public void RunOnce() { bool lockTaken = Monitor.TryEnter(_locker); if (!lockTaken) return; try { // Run code that should be executed by one thread only. } finally { Monitor.Exit(_locker); } }

...or the SemaphoreSlim class if you prefer to prevent reentrancy:

SemaphoreSlim _semaphore = new(1, 1); public void RunOnce() { bool lockTaken = _semaphore.Wait(0); if (!lockTaken) return; try { // Run code that should be executed by one thread only. } finally { _semaphore.Release(); } }

It makes the intentions of your code cleaner IMHO.

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

@Servy I mean at any given moment, only one thread will be allowed to execute the code in the protected section. Do you think that the current abbreviated comment is misleading?
It's not misleading, it's just inaccurate. Any number of threads can be executing that code at the same time. The comment is true when using an actual synchronization mechanisms, as they provide such guarantees, but the lock free code doesn't provide that guarantee. That's exactly why so many people caution its use, the restrictions it provides on how the code can be reordered are much weaker.
@Servy I am sorry, I am not going to write an infinite number of programs in order to prove that my lock-free implementation guarantees exclusive execution of the protected region. What I am going to do is post this link: Does Interlocked.CompareExchange use a memory barrier?, which covers quite nicely your code-reordering considerations IMHO. Finally I would like to thank you for sharing your knowledge with us.
@Dennis19901 the reason for the CompareExchange in the finally block is the same reason that the SemaphoreSlim throws a SemaphoreFullException when it is released more times than it is acquired. It is for detecting software bugs. As for using Interlocked.Exchange and Volatile.Write instead of CompareExchange, now that I am thinking of it, in this specific case actually you can. The code in your question is thread-safe IMHO.
@TheodorZoulias Thanks. You make a good point about preventing the "release" multiple times.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.