[core] refactor ThreadLocal backendupstream/refactor-thread-local

3 files changed, 64 insertions, 82 deletions

diff --git a/platform/default/src/mbgl/util/thread_local.cpp b/platform/default/src/mbgl/util/thread_local.cpp
index 068c2880c4..5fb66c3440 100644
--- a/platform/default/src/mbgl/util/thread_local.cpp
+++ b/platform/default/src/mbgl/util/thread_local.cpp
@@ -1,66 +1,46 @@
 #include <mbgl/util/thread_local.hpp>
-
-#include <mbgl/gfx/backend_scope.hpp>
 #include <mbgl/util/logging.hpp>
-#include <mbgl/util/run_loop.hpp>
 
-#include <stdexcept>
 #include <cassert>
+#include <cstdlib>
 
 #include <pthread.h>
 
 namespace mbgl {
 namespace util {
-
-template <class T>
-class ThreadLocal<T>::Impl {
-public:
-    pthread_key_t key;
-};
-
-template <class T>
-ThreadLocal<T>::ThreadLocal() : impl(std::make_unique<Impl>()) {
-    int ret = pthread_key_create(&impl->key, [](void *) {});
-
-    if (ret) {
-        throw std::runtime_error("Failed to init local storage key.");
+namespace impl {
+
+ThreadLocalBase::ThreadLocalBase() {
+    static_assert(sizeof(storage) >= sizeof(pthread_key_t), "storage is too small");
+    static_assert(alignof(decltype(storage)) % alignof(pthread_key_t) == 0, "storage is incorrectly aligned");
+    if (pthread_key_create(&reinterpret_cast<pthread_key_t&>(storage), nullptr) != 0) {
+        Log::Error(Event::General, "Failed to initialize thread-specific storage key");
+        abort();
     }
 }
 
-template <class T>
-ThreadLocal<T>::~ThreadLocal() {
-    // ThreadLocal will not take ownership
-    // of the pointer it is managing. The pointer
-    // needs to be explicitly cleared before we
-    // destroy this object.
+ThreadLocalBase::~ThreadLocalBase() {
+    // ThreadLocal will not take ownership of the pointer it is managing. The pointer
+    // needs to be explicitly cleared before we destroy this object.
     assert(!get());
 
-    if (pthread_key_delete(impl->key)) {
-        Log::Error(Event::General, "Failed to delete local storage key.");
-        assert(false);
+    if (pthread_key_delete(reinterpret_cast<pthread_key_t&>(storage)) != 0) {
+        Log::Error(Event::General, "Failed to delete thread-specific storage key");
+        abort();
     }
 }
 
-template <class T>
-T* ThreadLocal<T>::get() {
-    auto* ret = reinterpret_cast<T*>(pthread_getspecific(impl->key));
-    if (!ret) {
-        return nullptr;
-    }
-
-    return ret;
+void* ThreadLocalBase::get() {
+    return pthread_getspecific(reinterpret_cast<pthread_key_t&>(storage));
 }
 
-template <class T>
-void ThreadLocal<T>::set(T* ptr) {
-    if (pthread_setspecific(impl->key, ptr)) {
-        throw std::runtime_error("Failed to set local storage.");
+void ThreadLocalBase::set(void* ptr) {
+    if (pthread_setspecific(reinterpret_cast<pthread_key_t&>(storage), ptr) != 0) {
+        Log::Error(Event::General, "Failed to set thread-specific storage");
+        abort();
     }
 }
 
-template class ThreadLocal<gfx::BackendScope>;
-template class ThreadLocal<Scheduler>;
-template class ThreadLocal<int>; // For unit tests
-
+} // namespace impl
 } // namespace util
 } // namespace mbgl
diff --git a/platform/qt/src/thread_local.cpp b/platform/qt/src/thread_local.cpp
index 4191195a86..a4277f0410 100644
--- a/platform/qt/src/thread_local.cpp
+++ b/platform/qt/src/thread_local.cpp
@@ -1,8 +1,5 @@
 #include <mbgl/util/thread_local.hpp>
 
-#include <mbgl/actor/scheduler.hpp>
-#include <mbgl/gfx/backend_scope.hpp>
-
 #include <array>
 #include <cassert>
 
@@ -10,40 +7,33 @@
 
 namespace mbgl {
 namespace util {
+namespace impl {
 
-template <class T>
-class ThreadLocal<T>::Impl {
-public:
-    QThreadStorage<std::array<T*, 1>> local;
-};
+// QThreadStorage tries to be smart and take ownership of the data, which we don't want. So we're
+// wrapping it in another type which doesn't own the pointer it contains.
+using StorageType = QThreadStorage<std::array<void*, 1>>;
 
-template <class T>
-ThreadLocal<T>::ThreadLocal() : impl(std::make_unique<Impl>()) {
-    set(nullptr);
+ThreadLocalBase::ThreadLocalBase() {
+    static_assert(sizeof(storage) >= sizeof(StorageType), "storage is too small");
+    static_assert(alignof(decltype(storage)) % alignof(StorageType) == 0, "storage is incorrectly aligned");
+    new (&reinterpret_cast<StorageType&>(storage)) QThreadStorage<void*>();
 }
 
-template <class T>
-ThreadLocal<T>::~ThreadLocal() {
-    // ThreadLocal will not take ownership
-    // of the pointer it is managing. The pointer
-    // needs to be explicitly cleared before we
-    // destroy this object.
+ThreadLocalBase::~ThreadLocalBase() {
+    // ThreadLocal will not take ownership of the pointer it is managing. The pointer
+    // needs to be explicitly cleared before we destroy this object.
     assert(!get());
+    reinterpret_cast<StorageType&>(storage).~QThreadStorage();
 }
 
-template <class T>
-T* ThreadLocal<T>::get() {
-    return impl->local.localData()[0];
+void* ThreadLocalBase::get() {
+    return reinterpret_cast<StorageType&>(storage).localData()[0];
 }
 
-template <class T>
-void ThreadLocal<T>::set(T* ptr) {
-   impl->local.localData()[0] = ptr;
+void ThreadLocalBase::set(void* ptr) {
+    reinterpret_cast<StorageType&>(storage).setLocalData({{ ptr }});
 }
 
-template class ThreadLocal<Scheduler>;
-template class ThreadLocal<gfx::BackendScope>;
-template class ThreadLocal<int>; // For unit tests
-
+} // namespace impl
 } // namespace util
 } // namespace mbgl
diff --git a/src/mbgl/util/thread_local.hpp b/src/mbgl/util/thread_local.hpp
index b0e26356b4..cf5579a2ba 100644
--- a/src/mbgl/util/thread_local.hpp
+++ b/src/mbgl/util/thread_local.hpp
@@ -1,29 +1,41 @@
 #pragma once
 
-#include <mbgl/util/noncopyable.hpp>
-
-#include <memory>
+#include <type_traits>
 
 namespace mbgl {
 namespace util {
+namespace impl {
+
+class ThreadLocalBase {
+protected:
+    ThreadLocalBase();
+    ~ThreadLocalBase();
+
+    void* get();
+    void set(void*);
+
+private:
+    std::aligned_storage_t<sizeof(void*), alignof(void*)> storage;
+};
+
+} // namespace impl
 
 template <class T>
-class ThreadLocal : public noncopyable {
+class ThreadLocal : public impl::ThreadLocalBase {
 public:
+    ThreadLocal() = default;
+
     ThreadLocal(T* val) {
-        ThreadLocal();
         set(val);
     }
 
-    ThreadLocal();
-    ~ThreadLocal();
-
-    T* get();
-    void set(T* ptr);
+    T* get() {
+        return reinterpret_cast<T*>(impl::ThreadLocalBase::get());
+    }
 
-private:
-    class Impl;
-    std::unique_ptr<Impl> impl;
+    void set(T* ptr) {
+        impl::ThreadLocalBase::set(ptr);
+    }
 };
 
 } // namespace util