[core] Make sure ThreadLocal will not own the pointer it is managing

ThreadLocal should not own the pointer it is managing because
the use case in Mapbox GL is to keep a pointer to a stack allocated
object, like:

```
MyObject foo;
threadLocal.set(&foo);
```

To keep consistency, it is required that we clear the managed
object before ThreadLocal gets destroyed by setting it to `nullptr`.

platform/default/thread_local.cpp

@@ -29,7 +29,11 @@ ThreadLocal<T>::ThreadLocal() : impl(std::make_unique<Impl>()) {
 
 template <class T>
 ThreadLocal<T>::~ThreadLocal() {
-    delete get();
+    // 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.");

platform/qt/src/thread_local.cpp

@@ -4,6 +4,7 @@
 #include <mbgl/map/backend_scope.hpp>
 
 #include <array>
+#include <cassert>
 
 #include <QThreadStorage>
 
@@ -23,7 +24,11 @@ ThreadLocal<T>::ThreadLocal() : impl(std::make_unique<Impl>()) {
 
 template <class T>
 ThreadLocal<T>::~ThreadLocal() {
-    delete get();
+    // 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());
 }
 
 template <class T>

scripts/valgrind.sup

@@ -76,11 +76,3 @@
    fun:_ZN4mbgl4util10write_fileERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES8_
    ...
 }
-{
-   Qt5 ThreadStorage
-   Memcheck:Leak
-   match-leak-kinds: definite
-   fun:_Znwm
-   fun:_Z24qThreadStorage_localDataISt5arrayIPiLm1EEERT_R18QThreadStorageDataPS3_.isra.5
-   ...
-}

test/util/thread_local.test.cpp

@@ -71,30 +71,38 @@ TEST(ThreadLocalStorage, NotSetReturnsNull) {
 
 namespace {
 
-class TestThreadReclaim {
+class TestThreadDataOwnership {
 public:
-    TestThreadReclaim(mbgl::ActorRef<TestThreadReclaim>, int* data_) {
+    TestThreadDataOwnership(mbgl::ActorRef<TestThreadDataOwnership>, int* data_) {
         data.set(data_);
     }
 
+    ~TestThreadDataOwnership() {
+        data.set(nullptr);
+    }
+
 private:
-    ThreadLocal<int> data;
+    static ThreadLocal<int> data;
 };
 
+ThreadLocal<int> TestThreadDataOwnership::data;
+
 } // namespace
 
-TEST(ThreadLocalStorage, AutoReclaim) {
+TEST(ThreadLocalStorage, ShouldNotTakeOwnership) {
     RunLoop loop;
 
-    auto data1 = new int;
-    auto data2 = new int;
+    auto data1 = std::make_unique<int>(10);
+    auto data2 = std::make_unique<int>(20);
 
-    auto thread1 = std::make_unique<Thread<TestThreadReclaim>>("Test", data1);
-    auto thread2 = std::make_unique<Thread<TestThreadReclaim>>("Test", data2);
+    auto thread1 = std::make_unique<Thread<TestThreadDataOwnership>>("Test", data1.get());
+    auto thread2 = std::make_unique<Thread<TestThreadDataOwnership>>("Test", data2.get());
 
     thread1.reset();
     thread2.reset();
 
-    // Should not leak, valgrind will
-    // let us know.
+    // Will crash if ThreadLocal destroys
+    // the pointer it is managing.
+    ASSERT_EQ(*data1, 10);
+    ASSERT_EQ(*data2, 20);
 }