summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--platform/default/thread_local.cpp6
-rw-r--r--platform/qt/src/thread_local.cpp7
-rw-r--r--scripts/valgrind.sup8
-rw-r--r--test/util/thread_local.test.cpp28
4 files changed, 29 insertions, 20 deletions
diff --git a/platform/default/thread_local.cpp b/platform/default/thread_local.cpp
index b754a04b7d..6fdb1e6dc1 100644
--- a/platform/default/thread_local.cpp
+++ b/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.");
diff --git a/platform/qt/src/thread_local.cpp b/platform/qt/src/thread_local.cpp
index e48a9d6e74..e835a680e2 100644
--- a/platform/qt/src/thread_local.cpp
+++ b/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>
diff --git a/scripts/valgrind.sup b/scripts/valgrind.sup
index f659122dc1..09e2f6685c 100644
--- a/scripts/valgrind.sup
+++ b/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
- ...
-}
diff --git a/test/util/thread_local.test.cpp b/test/util/thread_local.test.cpp
index 12a19ab59b..7142697f48 100644
--- a/test/util/thread_local.test.cpp
+++ b/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);
}