[core] Isolate pthread-based tls implementation

8 files changed, 88 insertions, 51 deletions

diff --git a/platform/android/config.cmake b/platform/android/config.cmake
index a7370da5fd..5a6950a23f 100644
--- a/platform/android/config.cmake
+++ b/platform/android/config.cmake
@@ -55,6 +55,7 @@ macro(mbgl_platform_core)
         PRIVATE platform/android/src/thread.cpp
         PRIVATE platform/default/string_stdlib.cpp
         PRIVATE platform/default/bidi.cpp
+        PRIVATE platform/default/thread_local.cpp
         PRIVATE platform/default/utf.cpp
 
         # Image handling
diff --git a/platform/default/thread_local.cpp b/platform/default/thread_local.cpp
new file mode 100644
index 0000000000..b754a04b7d
--- /dev/null
+++ b/platform/default/thread_local.cpp
@@ -0,0 +1,62 @@
+#include <mbgl/util/thread_local.hpp>
+
+#include <mbgl/map/backend_scope.hpp>
+#include <mbgl/util/logging.hpp>
+#include <mbgl/util/run_loop.hpp>
+
+#include <stdexcept>
+#include <cassert>
+
+#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.");
+    }
+}
+
+template <class T>
+ThreadLocal<T>::~ThreadLocal() {
+    delete get();
+
+    if (pthread_key_delete(impl->key)) {
+        Log::Error(Event::General, "Failed to delete local storage key.");
+        assert(false);
+    }
+}
+
+template <class T>
+T* ThreadLocal<T>::get() {
+    auto* ret = reinterpret_cast<T*>(pthread_getspecific(impl->key));
+    if (!ret) {
+        return nullptr;
+    }
+
+    return ret;
+}
+
+template <class T>
+void ThreadLocal<T>::set(T* ptr) {
+    if (pthread_setspecific(impl->key, ptr)) {
+        throw std::runtime_error("Failed to set local storage.");
+    }
+}
+
+template class ThreadLocal<RunLoop>;
+template class ThreadLocal<BackendScope>;
+template class ThreadLocal<int>; // For unit tests
+
+} // namespace util
+} // namespace mbgl
diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake
index fdb286a6d1..401a0e202a 100644
--- a/platform/ios/config.cmake
+++ b/platform/ios/config.cmake
@@ -41,6 +41,7 @@ macro(mbgl_platform_core)
         PRIVATE platform/darwin/src/nsthread.mm
         PRIVATE platform/darwin/src/string_nsstring.mm
         PRIVATE platform/default/bidi.cpp
+        PRIVATE platform/default/thread_local.cpp
         PRIVATE platform/default/utf.cpp
 
         # Image handling
diff --git a/platform/linux/config.cmake b/platform/linux/config.cmake
index 41e7f71b99..d16842c201 100644
--- a/platform/linux/config.cmake
+++ b/platform/linux/config.cmake
@@ -69,6 +69,7 @@ macro(mbgl_platform_core)
         PRIVATE platform/default/string_stdlib.cpp
         PRIVATE platform/default/thread.cpp
         PRIVATE platform/default/bidi.cpp
+        PRIVATE platform/default/thread_local.cpp
         PRIVATE platform/default/utf.cpp
 
         # Image handling
diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake
index 8dc3c38245..e0edc90a6a 100644
--- a/platform/macos/config.cmake
+++ b/platform/macos/config.cmake
@@ -37,6 +37,7 @@ macro(mbgl_platform_core)
         PRIVATE platform/darwin/src/nsthread.mm
         PRIVATE platform/darwin/src/string_nsstring.mm
         PRIVATE platform/default/bidi.cpp
+        PRIVATE platform/default/thread_local.cpp
         PRIVATE platform/default/utf.cpp
 
         # Image handling
diff --git a/platform/qt/qt.cmake b/platform/qt/qt.cmake
index e5b96bfe93..9febee88b5 100644
--- a/platform/qt/qt.cmake
+++ b/platform/qt/qt.cmake
@@ -36,6 +36,9 @@ set(MBGL_QT_FILES
     PRIVATE platform/default/mbgl/util/default_thread_pool.cpp
     PRIVATE platform/default/mbgl/util/default_thread_pool.hpp
 
+    # Thread
+    PRIVATE platform/default/thread_local.cpp
+
     # Platform integration
     PRIVATE platform/qt/src/async_task.cpp
     PRIVATE platform/qt/src/async_task_impl.hpp
diff --git a/src/mbgl/util/thread_local.hpp b/src/mbgl/util/thread_local.hpp
index 15d4d56524..b0e26356b4 100644
--- a/src/mbgl/util/thread_local.hpp
+++ b/src/mbgl/util/thread_local.hpp
@@ -1,12 +1,8 @@
 #pragma once
 
-#include <mbgl/util/logging.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
-#include <cassert>
-#include <stdexcept>
-
-#include <pthread.h>
+#include <memory>
 
 namespace mbgl {
 namespace util {
@@ -14,40 +10,20 @@ namespace util {
 template <class T>
 class ThreadLocal : public noncopyable {
 public:
-    ThreadLocal() {
-        int ret = pthread_key_create(&key, [](void *ptr) {
-            delete reinterpret_cast<T *>(ptr);
-        });
-
-        if (ret) {
-            throw std::runtime_error("Failed to init local storage key.");
-        }
-    }
-
-    ~ThreadLocal() {
-        if (pthread_key_delete(key)) {
-            Log::Error(Event::General, "Failed to delete local storage key.");
-            assert(false);
-        }
+    ThreadLocal(T* val) {
+        ThreadLocal();
+        set(val);
     }
 
-    T* get() {
-        auto* ret = reinterpret_cast<T*>(pthread_getspecific(key));
-        if (!ret) {
-            return nullptr;
-        }
+    ThreadLocal();
+    ~ThreadLocal();
 
-        return ret;
-    }
-
-    void set(T* ptr) {
-        if (pthread_setspecific(key, ptr)) {
-            throw std::runtime_error("Failed to set local storage.");
-        }
-    }
+    T* get();
+    void set(T* ptr);
 
 private:
-    pthread_key_t key;
+    class Impl;
+    std::unique_ptr<Impl> impl;
 };
 
 } // namespace util
diff --git a/test/util/thread_local.test.cpp b/test/util/thread_local.test.cpp
index 0590e8b4dc..12a19ab59b 100644
--- a/test/util/thread_local.test.cpp
+++ b/test/util/thread_local.test.cpp
@@ -71,38 +71,30 @@ TEST(ThreadLocalStorage, NotSetReturnsNull) {
 
 namespace {
 
-struct DtorCounter {
-    ~DtorCounter() { ++(*value); }
-    unsigned *value;
-};
-
 class TestThreadReclaim {
 public:
-    TestThreadReclaim(mbgl::ActorRef<TestThreadReclaim>, DtorCounter* counter_) {
-        counter.set(counter_);
+    TestThreadReclaim(mbgl::ActorRef<TestThreadReclaim>, int* data_) {
+        data.set(data_);
     }
 
 private:
-    static ThreadLocal<DtorCounter> counter;
+    ThreadLocal<int> data;
 };
 
-ThreadLocal<DtorCounter> TestThreadReclaim::counter;
-
 } // namespace
 
 TEST(ThreadLocalStorage, AutoReclaim) {
     RunLoop loop;
 
-    unsigned counter = 0;
-
-    auto dtorCounter1 = new DtorCounter{ &counter };
-    auto dtorCounter2 = new DtorCounter{ &counter };
+    auto data1 = new int;
+    auto data2 = new int;
 
-    auto thread1 = std::make_unique<Thread<TestThreadReclaim>>("Test", dtorCounter1);
-    auto thread2 = std::make_unique<Thread<TestThreadReclaim>>("Test", dtorCounter2);
+    auto thread1 = std::make_unique<Thread<TestThreadReclaim>>("Test", data1);
+    auto thread2 = std::make_unique<Thread<TestThreadReclaim>>("Test", data2);
 
     thread1.reset();
     thread2.reset();
 
-    EXPECT_EQ(counter, 2u);
+    // Should not leak, valgrind will
+    // let us know.
 }