Using a variable in multithreaded environment - volatile, mutex or what?

What are you trying to achieve? When you have just a single variable multiple readers and multiple writers is suspicious.

I'm curious...what if you set the debug version of multi threaded library switch in project properties release configuration. Does it still 'stop working'? Might narrow down what's going on here.

What does 'not working' mean in your context?

If you have a mutex around writing the variable, but not reading, the only change you made in synchronization is that no multiple writers can write to the variable. However, it is still possible for a reader to read the variable halfway through the write operation. If that variable is not an atomic type (e.g. a struct or other compound), your read operation may very well happen halfway that write.

Surrounding your read with a lock/unlock would solve that problem, since you now guarantee that no two threads may be in the parts surrounded by lock/unlock simultaneously.

Protect the member behind accessor functions. Lock/Unlock on both get and set. Sleep/Retry with a threshold if locked.

Volatile in threaded code is wrong (it hinders the optimiser, yes, but does not guarantee correctness). Memory ordering is the big issue. Typically you have a pattern where you do some work, then set a flag to indicate to another thread that the work is done. For this to be correct, the "work" needs to arrive in RAM before the "flag" does. Volatile in no way ensures that this condition is met (except by luck half the time).

The simplest method is to just wrap all uses of the flag (read and write) in a lock (mutex, etc) as, assuming the lock is written by someone qualified, the lock will use the appropriate CPU instructions to ensure that memory access occurs in the appropriate order.

If this is actually a performance problem (locks are typically really fast!!) the next lower step down is std::atomic, not volatile. But again, it can be a bit dangerous if you're not familiar with the underlying memory model. The next step down from that, your compiler will have intrinsic functions that can be used to emit the necessary CPU memory barrier instructions directly, and intrinsics to disable the specific compiler optimisations that could interfere with your ordering requirements. The next step down from that is writing assembly for a specific CPU architecture

If you see volatile in your code, you have a bug.

10 hours ago, Hodgman said:

If you see volatile in your code, you have a bug. 

While I understand what you're aiming at, in the absolute way you state this, it's not right. There are circumstances where volatile is the right thing to use. It is unlikely for a game developer on modern hardware to ever be in such circumstances, but there are areas of software engineering, where it's the right thing to do (embedded/low level software, retro hardware games). Basically whenever dealing with memory that is not under the control of your program (hardware registers, memory mapped IO).

If you see a volatile in high level code for modern hardware, you most likely have a bug ?

volatile is must-have keyword when working without std::atomic but hardware atomic functions like InterlockedExchange on MSVC or __sync_lock_test_and_set in GCC/clang so this depends.

When there is need for reading/ writing protection and the operation is just setting a variable, I use an atomic SpinLock or ReadWriteSpinLock if performance really matters. In case of atomic types like unsigned int you could also consider using atomic operations directly instead of locking.

If the operation is considered to longer runs then using Mutex is absolutely ok but keep in mind that Mutexes in Windows are protected from OS to prevent self locking and you need to use Semaphore instead. I use this for example in my Task System to let idle threads go to sleep by their own.

You should always ensure to use the same lock for read and write or the effect will be pointless

1 hour ago, Shaarigan said:

volatile is must-have keyword when working without std::atomic but hardware atomic functions like InterlockedExchange on MSVC or __sync_lock_test_and_set in GCC/clang so this depends.

Yeah these are the intrinsics I was referencing on some platforms they want you to use pointers to volatile, but that's out of your control. On other platforms use non-volatile pointers. This is a level below std::atomic though - if you're writing code at this level, you're targeting a particular compiler and probably a particular CPU architectures. You have to aware of the memory model for that architecture and you're likely aware of the ASM that you're using the intrinsics to generate. At this level, you should basically just implement something like std::atomic and then use that. If you ever see it in gamedev outside of somewhere where an API has forced it upon you, be very suspicious.

2 hours ago, rnlf_in_space said:

While I understand what you're aiming at, in the absolute way you state this, it's not right. There are circumstances where volatile is the right thing to use. It is unlikely for a game developer on modern hardware to ever be in such circumstances, but there are areas of software engineering, where it's the right thing to do (embedded/low level software, retro hardware games). Basically whenever dealing with memory that is not under the control of your program (hardware registers, memory mapped IO).

Yeah talking to hardware is probably the only exception. My hyperbole ("volatile is a bug") is a famous quote from the Linux kernel memo "volatile considered harmful", where they ban it from use in the kernel - even something that low level doesn't need it. Hardware device drivers are beyond the scope of the kernel though, and likely will have valid uses.

However, if you're talking to hardware that can also access RAM and you communicate via MMIO and shared memory, you still need to be aware of the underlying memory model and also emit the appropriate fence instructions to ensure that your memory visibility and IO writes occur in the intended order.

40 minutes ago, Hodgman said:

on some platforms they want you to use pointers to volatile, but that's out of your control. On other platforms use non-volatile pointers. This is a level below std::atomic though - if you're writing code at this level, you're targeting a particular compiler and probably a particular CPU architectures. You have to aware of the memory model for that architecture and you're likely aware of the ASM that you're using the intrinsics to generate. At this level, you should basically just implement something like std::atomic and then use that. If you ever see it in gamedev outside of somewhere where an API has forced it upon you, be very suspicious.

I use that behind the scenes as platform and architecture independent std::atomic replacement for something like spin locking in our professional engine

#if defined(__GNUC__)
#define SpinLock(__lock) { while (sync_lock_test_and_set(&(__lock), 1)) while (lock) {} }
#define SpinUnlock(__lock) { __sync_lock_release(&(__lock)); }
#elif defined(WINDOWS)
#define SpinLock(__lock) { while (InterlockedExchange(&(__lock), 1)) while (__lock) {} }
#define SpinUnlock(__lock) { InterlockedExchange(&(__lock), 0); }
#endif

But anybody should stay aware of exotic platforms that implement their own interlocked functions like PSSDK or Switch SDK does

14 hours ago, Shaarigan said:

I use that behind the scenes as platform and architecture independent std::atomic replacement for something like spin locking in our professional engine

#if defined(__GNUC__)
#define SpinLock(__lock) { while (sync_lock_test_and_set(&(__lock), 1)) while (lock) {} }
#define SpinUnlock(__lock) { __sync_lock_release(&(__lock)); }
#elif defined(WINDOWS)
#define SpinLock(__lock) { while (InterlockedExchange(&(__lock), 1)) while (__lock) {} }
#define SpinUnlock(__lock) { InterlockedExchange(&(__lock), 0); }
#endif

But anybody should stay aware of exotic platforms that implement their own interlocked functions like PSSDK or Switch SDK does

While we're on the topic, Intel recommends that you put a _mm_pause or YieldProcessor (MSVC) call inside the body of any spin loop, which emits a special kind of NOP instruction. The CPU will recognize this pattern, de-pipeline the decoded loop instructions, switch to the HW hyper-thread if the loop keeps spinning, and reduce power consumption until the loop exits

For everyone else: using spin-loops is in general a pretty bad idea for performance. Your compiler's default locks (e.g. a critical section on Win32/MSVC) will use a tried, tested and well tuned spin-lock and then progressively fall back to more heavyweight algorithms if it spins for "too long".

First of all thanks for all your answers.

Please correct me if I'm drawing bad conclusions from it but I think that in a case where thread 1 is only writing and thread 2 is only reading then the only purpose of mutex lock/unlock is to prevent compiler from reordering instructions and making wrong optimizations. Is that correct? Because precisely what I'm doing with my bool variable is to mark the moment when some work has been done. So I wouldn't really want my compiler to set that variable to true *before* the actual work is done.

If the above is correct then it doesn't really matter if thread 2 reads memory while thread 1 is halfway writing to it, or does it? Assuming thread 2 is waiting for the variable to be true, then if thread 1 is writing (changing variable from false to true) the thread 2 doesn't really care when it reads true eventually, right?

@Shaarigan Could you elaborate a bit more on "If the operation is considered to longer runs then using Mutex is absolutely ok but keep in mind that Mutexes in Windows are protected from OS to prevent self locking and you need to use Semaphore instead"? In what case would semaphore work whereas mutex would not (on Windows)?

5 minutes ago, maxest said:

Please correct me if I'm drawing bad conclusions from it but I think that in a case where thread 1 is only writing and thread 2 is only reading then the only purpose of mutex lock/unlock is to prevent compiler from reordering instructions and making wrong optimizations. Is that correct? Because precisely what I'm doing with my bool variable is to mark the moment when some work has been done. So I wouldn't really want my compiler to set that variable to true *before* the actual work is done.

Yep. But not JUST the compiler. Some CPUs will reorder your instructions at runtime (e.g. Intel), some will reorder memory reads and writes (Intel in certain situations), some will do both! Low level ASM/binary instructions are actually basically a high level byte code these days, and modern CPUs will take that instruction stream and dynamically compile it into another set of internal instructions... Optimising on the fly

So, you do some work, make sure that the work is actually completed and visible to other CPU cores, then write the boolean. The lock will take care of that platform-specific CPU ordering, cache flushing, RAM visibility nonsense, with compile-time hints, and runtime instructions, if necessary on your current platform.

In this particular situation, you could also just use a std::atomic instead of a lock+boolean -- they have functions that let you write a value after also performing a memory-fence operation.

5 minutes ago, maxest said:

If the above is correct then it doesn't really matter if thread 2 reads memory while thread 1 is halfway writing to it, or does it? Assuming thread 2 is waiting for the variable to be true, then if thread 1 is writing (changing variable from false to true) the thread 2 doesn't really care when it reads true eventually, right?

Due to CPUs reordering things, thread 2 might read the work and then the boolean, or thread 1 might write the boolean and then write the work. Either of those situations would cause thread 2 to process old/uninitialized data instead of thread 1's actual work output. Memory fences (internally handled by locking primitives and std::atomic) make sure that this kind of reordering won't occur (both at compile time and at execution time within the CPU).

@maxset

I have had a concrete usecase for a self-blocking lock in my ThreadPool/ TaskScheduler. Idle threads should themselves go to sleep they wont burn any CPU time and I first implemented this on a self-locking mutex. Debug build worked well but I got failure in Release.

In the end, Mutexes in Windows are designed to some kind of self locking detection while Semaphore isn't so this was my remark to this topic

Thanks Hodgman.

@Shaarigan Yeah, I also once used semaphore instead of a mutex because on Semaphore Acquire operation the CPU is yielded on that thread. So by saying " you need to use Semaphore instead " you meant it was more optimal this way, not that using semaphore was correct in that case whereas using a mutex was not?

To achieve what I described above you need! to use Semaphore at least on Windows because as I wrote Mutex on Windows is protected against self locking. So a thread holding the lock already can't rely to be set to sleep when it will aquire the same lock on the same thread again.

This was my case because each Thread holds it's own lock in locking state and if it would get idle instead of burning cycles, it aquires the same lock again to deadlock itself and go to sleep indefinitely. Another process then will check the lock-state and fire a release so the idle thread is notified that there is work to do again.

This was my personal usecase for that :)

On 10/2/2018 at 4:02 AM, Shaarigan said:

#define SpinLock(__lock) { while (sync_lock_test_and_set(&(__lock), 1)) while (lock) {} } // ERROR: "lock" undefined.

L. Spiro

Keep the bug, it's a giveaway

@Shaarigan Understood what you mean. I wasn't even aware that you can mutex lock twice on Windows and it's basically non-blocking when happens on the same thread. It would be logical for me for a thread to go to Sleep upon the second lock. But I think it would be a bad practice to implement self-locking this way. Using semaphore is way more elegant to me. I used semaphore as well for keeping track of the number of jobs to be processed by a job system. If anyone is interested in taking a look at how to make (a simple) one, or pinpoint some issues :), check here:
https://github.com/maxest/MaxestFramework/blob/master/src/common/jobs.h
https://github.com/maxest/MaxestFramework/blob/master/src/common/jobs.cpp