Skip to content

Commit 6e41d6c

Browse files
committed
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.
1 parent d1b367f commit 6e41d6c

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

platform/SingletonPtr.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <stdint.h>
2929
#include <new>
3030
#include "platform/mbed_assert.h"
31+
#include "platform/mbed_critical.h"
3132
#ifdef MBED_CONF_RTOS_PRESENT
3233
#include "cmsis_os2.h"
3334
#endif
@@ -92,17 +93,20 @@ struct SingletonPtr {
9293
*/
9394
T *get() const
9495
{
95-
if (NULL == _ptr) {
96+
T *p = static_cast<T *>(core_util_atomic_load_ptr(&_ptr));
97+
if (p == NULL) {
9698
singleton_lock();
97-
if (NULL == _ptr) {
98-
_ptr = new (_data) T();
99+
p = static_cast<T *>(_ptr);
100+
if (p == NULL) {
101+
p = new (_data) T();
102+
core_util_atomic_store_ptr(&_ptr, p);
99103
}
100104
singleton_unlock();
101105
}
102106
// _ptr was not zero initialized or was
103107
// corrupted if this assert is hit
104-
MBED_ASSERT(_ptr == (T *)&_data);
105-
return _ptr;
108+
MBED_ASSERT(p == reinterpret_cast<T *>(&_data));
109+
return p;
106110
}
107111

108112
/** Get a pointer to the underlying singleton
@@ -126,7 +130,7 @@ struct SingletonPtr {
126130
}
127131

128132
// This is zero initialized when in global scope
129-
mutable T *_ptr;
133+
mutable void *_ptr;
130134
#if __cplusplus >= 201103L
131135
// Align data appropriately
132136
alignas(T) mutable char _data[sizeof(T)];

0 commit comments

Comments
 (0)