Make SingletonPtr safe using atomics

SingletonPtr's implementation wasn't totally safe - see "C++ and the
Perils of Double-Checked Locking"by Meyers and Alexandrescu. No problems
observed in practice, but it was potentially susceptible to compiler
optimisation (or maybe even SMP issues).

Now that we have atomic loads and stores, the function can be made safe,
avoiding any potential races for threads that don't take the lock:
ensure that the unlocked load is atomic, and that the pointer store is
atomic.

See https://preshing.com/20130930/double-checked-locking-is-fixed-in-cpp11/
for more discussion.
pull/9530/head
Kevin Bracey 2019-01-29 11:14:02 +02:00
parent d1b367fbab
commit 6e41d6cdb7
1 changed files with 10 additions and 6 deletions

View File

@ -28,6 +28,7 @@
#include <stdint.h>
#include <new>
#include "platform/mbed_assert.h"
#include "platform/mbed_critical.h"
#ifdef MBED_CONF_RTOS_PRESENT
#include "cmsis_os2.h"
#endif
@ -92,17 +93,20 @@ struct SingletonPtr {
*/
T *get() const
{
if (NULL == _ptr) {
T *p = static_cast<T *>(core_util_atomic_load_ptr(&_ptr));
if (p == NULL) {
singleton_lock();
if (NULL == _ptr) {
_ptr = new (_data) T();
p = static_cast<T *>(_ptr);
if (p == NULL) {
p = new (_data) T();
core_util_atomic_store_ptr(&_ptr, p);
}
singleton_unlock();
}
// _ptr was not zero initialized or was
// corrupted if this assert is hit
MBED_ASSERT(_ptr == (T *)&_data);
return _ptr;
MBED_ASSERT(p == reinterpret_cast<T *>(&_data));
return p;
}
/** Get a pointer to the underlying singleton
@ -126,7 +130,7 @@ struct SingletonPtr {
}
// This is zero initialized when in global scope
mutable T *_ptr;
mutable void *_ptr;
#if __cplusplus >= 201103L
// Align data appropriately
alignas(T) mutable char _data[sizeof(T)];