3

I've made a hand-made mutex for my project, but I doubt if it is thread safe...

    bool blocked;

    while ( blocked )
    {

    }

    blocked = true;
    ...
    blocked = false;

Lets say, thread A passes the while loop and doesn't block the flag in time (doesn't have time to set the flag to false) and thread B passes the while loop too!

  1. Is it possible? Why?

  2. As I get it mutex has the very same principle of work. Why can not this occur in mutex? I've read about atomic operations which can not be interrupted... So the check-if-mutex-available and mutex-block can not be interrupted, right?

Kolyunya
  • 5,973
  • 7
  • 46
  • 81

5 Answers5

8

Your code is completely defunct!

The reason is that access to the variable blocked is not atomic. Two threads can read it si­mul­ta­ne­ous­ly and decide that the mutex is unlocked, if the two reads happen before the first thread writes out the true update and the update propagates to all CPUs.

You need atomic variables and atomic exchange to solve this. The atomic_flag type is exactly what you want:

#include <atomic>

std::atomic_flag blocked;

while (blocked.test_and_set()) { }  // spin while "true"

// critical work goes here

blocked.clear();                    // unlock

(Alternatively, you could use a std::atomic<bool> and exchange(true), but the atomic_flag is made especially for this purpose.)

Atomic variables do not only prevent the compiler from reordering code that appears to be unrelated if this were a single-threaded context, but they also make the compiler generate the necessary code to prevent the CPU itself from reordering instructions in a way that would allow inconsistent execution flow.

In fact, if you want to be slightly more efficient, you can demand a less expensive memory ordering on the set and clear operations, like so:

while (blocked.test_and_set(std::memory_order_acquire)) { }  // lock

// ...

blocked.clear(std::memory_order_release);                    // unlock

The reason is that you only care about the correct ordering in one direction: a delayed update in the other direction is not very expensive, but requiring sequential consistency (as is the default) may well be expensive.

Important: The above code is a so-called spin lock, because while the state is locked, we do a busy spin (the while loop). This is very bad in almost all situations. A kernel-provided mutex system call is a whole different kettle of fish, since it allows the thread to signal the kernel that it can go to sleep and let the kernel deschedule the entire thread. That is almost always the better behaviour.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 2
    Still this solution is not exception safe. What if 'critical work' will throw an exception? Just use std::mutex or boost::mutex, their implementation is fast enough – nogard Sep 11 '12 at 09:57
  • Mmh, shouldn't it be `while (blocked.test_and_set()); do_critical_work(); blocked.clear();`? – akappa Sep 11 '12 at 09:58
  • @nogard: No, it is not exception safe. Neither is a mutex. – Kerrek SB Sep 11 '12 at 10:00
  • @KerrekSB mutex::lock IS exception safe, because when exception is thrown -> stack unwinds -> lock is destroyed -> mutex is unlocked. This is the main purpose of the lock guards. – nogard Sep 11 '12 at 10:03
  • @nogard: You're conflating mutexes and lock-guards! Always separate your responsibilities... – Kerrek SB Sep 11 '12 at 10:04
  • @KerrekSB: lock guard is the best practice to lock the mutex. In most cases you should not call mutex.lock() or mutex.unlock() explicitly. So if you use mutex properly with lock guard it IS exception safe, while your solution is not. Spin locks could be used for really short operations like assigning variables, but not for 'critical work' – nogard Sep 11 '12 at 10:09
  • Thanks much for a reply. And what about `pthread_mutex_t`? Wanted to use __them__. Are they ok? – Kolyunya Sep 11 '12 at 10:09
  • 1
    @nogard: You can easily create a lock guard like wrapper around his code, where is the problem? He describes just exactly a homebrewn wannabe mutex, not more. Just like if you would replace his description by the internal implementation details of boost mutex, it would be no different (exception safety wise). – PlasmaHH Sep 11 '12 at 10:39
  • @nogard: No, you should separate the synchronisation primitive (which I present in this post) which is associated to the *shared data*, from the act of *locking* said primitive, which is associated to a *thread*. A per-thread lock guard object is easily made; in fact, just use `std::lock_guard` and add `lock()` and `unlock()` functions to my atomic construction. – Kerrek SB Sep 11 '12 at 10:43
  • @Kolyunya: A `pthread_mutex_t` is by far the preferable low-level solution. (What I described is essentially a `pthread_spinlock_t`.) You just call `pthread_mutex_lock`/`pthread_mutex_unlock`; the atomicity and reordering guarantees are provided by the library. Or use `std::mutex`, which is essentially a thin wrapper around that (on typical Posix systems). – Kerrek SB Sep 11 '12 at 10:46
  • @KerrekSB I don't get it, where is `atomic` header stored? I only found `cstdatomic` in `c++0x` – Kolyunya Sep 12 '12 at 08:12
  • line `#include ` causes an `error: atomic: No such file or directory`. What am I doing wrong? g++ 4.4 – Kolyunya Sep 12 '12 at 08:42
  • @Kolyunya: Are you adding `-std=c++0x`? Otherwise, upgrade to a more recent compiler. C++11 support is still evolving. GCC 4.6.3 does a good job with it. – Kerrek SB Sep 12 '12 at 09:04
3
  1. It's possible because the processor can switch from the first thread to the second after entering the loop, but before locking the mutex.
  2. It's possible because they use kernel-level operations to ensure that the thread is not switched until a certain operation is complete.

On Windows for instance, you could make a mutex look like this.

SingerOfTheFall
  • 29,228
  • 8
  • 68
  • 105
weltensturm
  • 2,164
  • 16
  • 17
  • your code requires non std functionality ... those atomic access functions are not even included – Walter Sep 11 '12 at 09:56
  • 2
    That's why "On Windows for instance". I don't know how far C++11 threading facilities are supported by todays compilers, so this was a logical choice. If one has to switch to Linux, it doesn't look much different there. – weltensturm Sep 11 '12 at 10:01
  • I wanted to use `pthread_mutex_t`. Is this implementation ok? I neither can use `boost` nor `c++11` – Kolyunya Sep 11 '12 at 10:14
  • Yes, it's the equivalent of what I posted on unix-like systems. – weltensturm Sep 11 '12 at 10:25
1

You've pretty much got it already.

  1. Yes, it's very possible. For a single core, threads get executed by the OS via timeslicing. It runs thread A for a bit, then pauses it, and runs thread B for a bit. Thread A could get paused right after passing the while loop.

  2. To solve problems like these, CPU's have implemented special instructions that CANNOT be interrupted by anything. These atomic operations are used by a mutex to check the flag, and set it in one operation.

GazTheDestroyer
  • 20,722
  • 9
  • 70
  • 103
1

Yes, the situation you described can happen. The reason for this is that the thread can be interrupted between testing blocked is false, and setting blocked to true. To get the behaviour you want you will need to use or emulate an atomic test-and-set operation.

Further information on test-and-set can be found here.

verdesmarald
  • 11,646
  • 2
  • 44
  • 60
  • the more serious reason is that it is not guaranteed that threads A and B see the same value for the variable `blocked`: it's not an atomic variable and there exit no synchronisation conditions between threads accessing it – Walter Sep 11 '12 at 09:58
1

a mutex implementation must ensure mutual exclusivity (that's the meaning of it) and not obtained in your code. It requires some atomic variable and suitable memory order for its access. In C++11, best use std::mutex (ideally together with std::lock), for C++03 you can use boost::mutex etc.

Walter
  • 44,150
  • 20
  • 113
  • 196
  • I wanted to use `pthread_mutex_t`. Is this implementation ok? I neither can use `boost` nor `c++11` – Kolyunya Sep 11 '12 at 10:13