From 6e41d6cdb7dbd5d9f2eb8338c40360c5e37fdf0a Mon Sep 17 00:00:00 2001
From: Kevin Bracey <kevin.bracey@arm.com>
Date: Tue, 29 Jan 2019 11:14:02 +0200
Subject: [PATCH] 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.
---
 platform/SingletonPtr.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/platform/SingletonPtr.h b/platform/SingletonPtr.h
index 82b7bcf50a..5cb109ea49 100644
--- a/platform/SingletonPtr.h
+++ b/platform/SingletonPtr.h
@@ -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)];