5

I was looking for an awaitable equivalent of lock statements in C#. Some people suggest using a binary SemaphoreSlim in the following way:

await semaphore.WaitAsync().ConfigureAwait(false);
try
{
    //inner instructions
}
finally
{
    semaphore.Release();
}

I know it has some issues (e.g. it's not reentrant), but my biggest concern is with the instruction reeordering.

In plain old lock statements we have a guarantee that no inner instruction from the lock will be moved outside (before or after) the lock statement. Does the same hold for this semaphore solution? As far as I can see, the documentation doesn't mention this problem.

tearvisus
  • 2,013
  • 2
  • 15
  • 32
  • 1
    I would like to know too, I tried to dig around and find this out a while ago and the best I could find was a *"I think it does..."* – Scott Chamberlain Dec 05 '16 at 17:34
  • 1
    Memory models on .NET are a complete mess. What's documented doesn't even match what's implemented. – Stephen Cleary Dec 19 '16 at 14:30
  • It's not being re-entrant isn't an issue, it's one place where it's superior to using `lock`. – Jon Hanna Dec 20 '16 at 09:59
  • @JonHanna: I know that lock reentrancy is a controversial behavior. What I meant is that this semaphore pattern behaves differently, so it fails to be an "equivalent of lock statements". Of course, there are other important differences, for example lack of thread affinity. – tearvisus Dec 20 '16 at 10:15
  • It's a close enough equivalent to `Debug.Assert(!Monitor.IsEntered(lockObj), "Lock re-entered!"); lock(lockObj){…}` ;) – Jon Hanna Dec 20 '16 at 11:44

3 Answers3

2

SemaphoreSlim, and pretty much all of the other synchronization constructs, are built using a Monitor (or other types that are built on top of a Monitor) internally, which is exactly how a lock is implemented, giving you the same guarantees.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • As far as I can see from the `SemaphoreSlim`'s source code, it uses Monitor/locks during waiting and releasing. But that does not guarantee yet much in our scenario, does it? For example, if there is a reading instruction between `semaphore.Wait()` and `semaphore.Release()` it could be moved before the `semaphore.Wait()` instruction. Am I right? – tearvisus Dec 05 '16 at 21:06
  • @tearvisus There is a monitor used in the wait, so no, it could not be. – Servy Dec 05 '16 at 21:07
  • According to the C# specification (point 3.10) reading a non-volatile variable doesn't have to be preserved at a critical execution point. Am I misunderstanding the specification or is there some other piece of documentation which prohibits this reordering? – tearvisus Dec 05 '16 at 21:13
  • @tearvisus If it could be then there would be no point in ever using a `lock`, if the code before or after a lock could be intermingled with the lock itself. – Servy Dec 05 '16 at 21:17
  • The code wouldn't be intermingled. A reading instruction after `semaphore.Wait()` (i.e. after semaphore's `Monitor.Exit()`) could end up before the `semaphore.Wait()` (i.e. before `Monitor.Enter()`). Nothing between `Monitor.Enter()` and `Monitor.Exit()` would change. At least as far as I understand the specification. – tearvisus Dec 05 '16 at 21:27
  • Since the problem of the 'intermingling' is a separate issue, I asked another [question](http://stackoverflow.com/questions/40991335/can-a-read-instruction-after-an-unrelated-lock-statement-be-moved-before-the-loc). – tearvisus Dec 06 '16 at 08:59
  • Not all (other) synchronization constructs use monitors. Some use operating system objects (e.g. `Semaphore`, `Mutex`, `ManualResetEvent`), others use `SpinWait` and `Interlocked` (e.g. `SpinLock`), some use architecture-specific operations (e.g. `Thread.MemoryBarrier()`). Even write and read on a `volatile` field is enough synchronization in specific cases. – acelent Dec 06 '16 at 09:18
  • Is there any official documentation that states this @Servy so we can rely on it as a contract or is this feature only a implementation detail and we don't have a guarantee that other runtimes (for example Mono) will give the same behavior? – Scott Chamberlain Dec 06 '16 at 16:05
0

The SemaphoreSlim guarantee is kind of implicit. It's described as a locking synchronization primitive in Overview of Synchronization Primitives.

acelent
  • 7,965
  • 21
  • 39
0

I am no expert in memory models, but now I think that we have those guarantees.

As Servy has pointed out, both the Wait and Release methods use a Monitor under the hood. However, a Monitor on its own may not be enough.

At the end of the Wait method, right before the Monitor.Exit call, a volatile field is decremented.

if (lockTaken)
{ 
    m_waitCount--; //m_waitCount is volatile
    Monitor.Exit(m_lockObj); 
}

As far as I understand, the decrement operator used on a volatile field will introduce both the 'acquire' and 'release' operations, blocking the following instructions from being reordered before it.

As for the Release method, the situation is analogous. At the beginning we have both the lock acquisition and volatile read-write operation as well.

lock (m_lockObj) 
{
    //m_currentCount is volatile
    if (m_maxCount - m_currentCount < releaseCount)
    {
        throw new SemaphoreFullException(); 
    }

    m_currentCount += releaseCount;

Special thanks to Joe Duffy for pointing out the importance of the volatile fields in the SemaphoreSlim.

EDIT: An example demonstrating a situation where the locks on their own (without additional volatile operations) may not be enough.

// Semaphore.Wait()
lock (syncRoot)
{
    // (1)
    // acquire semaphore
}
// end of Semaphore.Wait()

// the critical section guarded by the 'semaphore lock' (2)

// Semaphore.Release()
lock (syncRoot)
{
    // release semaphore
}
// end of Semaphore.Release()

A read instruction from the critical section (2) could be reordered to (1), when the semaphore is not yet acquired (another thread might still be working in a critical section).

Community
  • 1
  • 1
tearvisus
  • 2,013
  • 2
  • 15
  • 32
  • 1
    *"However, a `Monitor` on its own [may not be enough](http://stackoverflow.com/a/40991480/1892138)."* -- please, don't generate further disinformation. The CLI specification states in §12.6.5: *"Acquiring a lock (`System.Threading.Monitor.Enter` or entering a synchronized method) shall implicitly perform a volatile read operation, and releasing a lock (`System.Threading.Monitor.Exit` or leaving a synchronized method) shall implicitly perform a volatile write operation. See §12.6.7."* – acelent Dec 14 '16 at 18:54
  • You must be refering to the source code of [`SemaphoreSlim`](https://referencesource.microsoft.com/#q=type%20SemaphoreSlim). Let's analyze its volatile fileds. `m_currentCount` is volatile, because `Wait(int, CancellationToken)` performs an optimistic spin wait loop while its value is zero (without locking) and because the `CurrentCount` property reads it. `m_waitCount` has no need to be volatile. `m_waitHandle` is initialized using [double-ckecked locking](https://en.wikipedia.org/wiki/Double-checked_locking). – acelent Dec 14 '16 at 18:54
  • @acelent: Why do you say I'm "generating disinformation"? `Monitor.Exit` generates a volatile write, so a read instruction after `Exit` could be reordered before `Exit` (assuming no other volatile operations). – tearvisus Dec 18 '16 at 10:30
  • 1
    The expected guarantees regarding synchronization primitives are acquire and release, at least for the ones that can be used like a lock. You probably want or need former documentation or proof. However, the statement I quoted is simply wrong. I give you that you used the term *may* not, instead of *is* not. But I would refrain from such statements until decent documentation or proof that monitors may not be *enough* for synchronization purposes. – acelent Dec 19 '16 at 02:15
  • You found out reads may be reordered to before `Monitor.Exit`, perhaps you weren't expecting it. Can you come up with one case where this is *not enough*, or wrong? Remember, `Monitor.Exit` doesn't guarantee that a waiting thread will pick up instantly, and that it may be the underlying hardware reordering the reads that come after the exit. – acelent Dec 19 '16 at 02:15
  • But note that a lock exit has release semantics, so whatever (1) does to acquire the semaphore happens before the lock exit. In other words, it would be equally correct, although not equally performant, to have a single lock statement around the critical section. Now, if the critical section requires synchronization for its own, and the semaphore is not being used for such purpose (i.e. a semaphore with max count != 1 to wake producers or consumers), you may need additional synchronization. – acelent Dec 20 '16 at 10:42
  • I re-read my comment, and by a single lock statement around the critical section, I meant a single lock statement around the acquire semaphore, critical section and release semaphore. – acelent Dec 20 '16 at 12:31
  • @acelent: I've think there's a misunderstanding. `(1)` is not semaphore acquisition. It's just a place between the location where the mutex is entered and the place where the semaphore is acquired. The point is: some instruction from the critical section could be reordered to this place, i.e. to the place where the semaphore is not yet taken (so two threads would be in the critical section). – tearvisus Dec 27 '16 at 08:57
  • There's no misunderstanding. The code you presented may block on `lock (syncRoot)` in case of contention. Inside the lock, a blocking waiting loop may happen (e.g. `Monitor.Wait`) in case the semaphore cannot be immediately acquired. When the semaphore is acquired, the lock is released and your critical section is executed. Its memory accesses may not be reordered to before the **lastest** `Monitor.Enter` or `Monitor.Wait` on `syncRoot` in the same thread. There really isn't a problem with memory accesses being reordered to between this point and `Monitor.Exit`. – acelent Jan 09 '17 at 10:13
  • Supposing a binary semaphore, other threads will observe an acquired semaphore and they shouldn't run the critical section. They should either block while the semaphore cannot be acquired, or in case you're using e.g. `SemaphoreSlim.Wait(Int32)` which returns a boolean, they should conditionally **skip** the critical section (e.g. `if (semaphore.Wait(0)) { /* critical section */ } else { /* perform other things, increase number of attempts, whatever */ }`). If you're not using a binary semaphore, then the code is not really a critical section, you may need further synchronization. – acelent Jan 09 '17 at 10:14
  • @acelent: "Its memory accesses may not be reordered to before the lastest Monitor.Enter or Monitor.Wait" - a read access **may** be reordered before `Monitor.Wait` and that's the situation I'm talking about. – tearvisus Jan 10 '17 at 06:45
  • 1
    [`Monitor.Wait`](https://msdn.microsoft.com/en-us/library/ateab679(v=vs.110).aspx) is equivalent to exiting the lock, performing a blocking wait and entering the lock. Memory accesses after `Monitor.Wait` cannot be reordered to before it, essentially it's as if `Monitor.Enter` was called before it returned (successfully, in the overloads with a timeout argument). – acelent Jan 10 '17 at 10:46