diff --git a/flow/export_node.cc b/flow/export_node.cc index f5440f5473ff2..318b52ff3cc27 100644 --- a/flow/export_node.cc +++ b/flow/export_node.cc @@ -11,9 +11,8 @@ namespace { using ExportNodeBindings = std::unordered_map>; -FML_THREAD_LOCAL fml::ThreadLocal tls_export_node_bindings([](intptr_t value) { - delete reinterpret_cast(value); -}); +FML_THREAD_LOCAL fml::ThreadLocalUniquePtr + tls_export_node_bindings; } // namespace @@ -27,13 +26,11 @@ ExportNode::ExportNode(zx::eventpair export_token) void ExportNode::Create(zx_koid_t id, zx::eventpair export_token) { // This GPU thread contains at least 1 ViewHolder. Initialize the per-thread // bindings. - if (tls_export_node_bindings.Get() == 0) { - tls_export_node_bindings.Set( - reinterpret_cast(new ExportNodeBindings())); + if (tls_export_node_bindings.get() == nullptr) { + tls_export_node_bindings.reset(new ExportNodeBindings()); } - auto* bindings = - reinterpret_cast(tls_export_node_bindings.Get()); + auto* bindings = tls_export_node_bindings.get(); FML_DCHECK(bindings); FML_DCHECK(bindings->find(id) == bindings->end()); @@ -43,16 +40,14 @@ void ExportNode::Create(zx_koid_t id, zx::eventpair export_token) { } void ExportNode::Destroy(zx_koid_t id) { - auto* bindings = - reinterpret_cast(tls_export_node_bindings.Get()); + auto* bindings = tls_export_node_bindings.get(); FML_DCHECK(bindings); bindings->erase(id); } ExportNode* ExportNode::FromId(zx_koid_t id) { - auto* bindings = - reinterpret_cast(tls_export_node_bindings.Get()); + auto* bindings = tls_export_node_bindings.get(); if (!bindings) { return nullptr; } diff --git a/flow/view_holder.cc b/flow/view_holder.cc index f7c5de93b198f..2baba58f67f60 100644 --- a/flow/view_holder.cc +++ b/flow/view_holder.cc @@ -11,9 +11,8 @@ namespace { using ViewHolderBindings = std::unordered_map>; -FML_THREAD_LOCAL fml::ThreadLocal tls_view_holder_bindings([](intptr_t value) { - delete reinterpret_cast(value); -}); +FML_THREAD_LOCAL fml::ThreadLocalUniquePtr + tls_view_holder_bindings; fuchsia::ui::gfx::ViewProperties ToViewProperties(float width, float height, @@ -65,13 +64,11 @@ void ViewHolder::Create(zx_koid_t id, BindCallback on_bind_callback) { // This GPU thread contains at least 1 ViewHolder. Initialize the per-thread // bindings. - if (tls_view_holder_bindings.Get() == 0) { - tls_view_holder_bindings.Set( - reinterpret_cast(new ViewHolderBindings())); + if (tls_view_holder_bindings.get() == nullptr) { + tls_view_holder_bindings.reset(new ViewHolderBindings()); } - auto* bindings = - reinterpret_cast(tls_view_holder_bindings.Get()); + auto* bindings = tls_view_holder_bindings.get(); FML_DCHECK(bindings); FML_DCHECK(bindings->find(id) == bindings->end()); @@ -82,16 +79,14 @@ void ViewHolder::Create(zx_koid_t id, } void ViewHolder::Destroy(zx_koid_t id) { - auto* bindings = - reinterpret_cast(tls_view_holder_bindings.Get()); + auto* bindings = tls_view_holder_bindings.get(); FML_DCHECK(bindings); bindings->erase(id); } ViewHolder* ViewHolder::FromId(zx_koid_t id) { - auto* bindings = - reinterpret_cast(tls_view_holder_bindings.Get()); + auto* bindings = tls_view_holder_bindings.get(); if (!bindings) { return nullptr; } diff --git a/fml/message_loop.cc b/fml/message_loop.cc index af3d73510281c..652fb659bd2c1 100644 --- a/fml/message_loop.cc +++ b/fml/message_loop.cc @@ -15,12 +15,10 @@ namespace fml { -FML_THREAD_LOCAL ThreadLocal tls_message_loop([](intptr_t value) { - delete reinterpret_cast(value); -}); +FML_THREAD_LOCAL ThreadLocalUniquePtr tls_message_loop; MessageLoop& MessageLoop::GetCurrent() { - auto* loop = reinterpret_cast(tls_message_loop.Get()); + auto* loop = tls_message_loop.get(); FML_CHECK(loop != nullptr) << "MessageLoop::EnsureInitializedForCurrentThread was not called on " "this thread prior to message loop use."; @@ -28,15 +26,15 @@ MessageLoop& MessageLoop::GetCurrent() { } void MessageLoop::EnsureInitializedForCurrentThread() { - if (tls_message_loop.Get() != 0) { + if (tls_message_loop.get() != nullptr) { // Already initialized. return; } - tls_message_loop.Set(reinterpret_cast(new MessageLoop())); + tls_message_loop.reset(new MessageLoop()); } bool MessageLoop::IsInitializedForCurrentThread() { - return tls_message_loop.Get() != 0; + return tls_message_loop.get() != nullptr; } MessageLoop::MessageLoop() diff --git a/fml/thread_local.cc b/fml/thread_local.cc index d0afbc00186a0..f997ecc05f68b 100644 --- a/fml/thread_local.cc +++ b/fml/thread_local.cc @@ -4,66 +4,32 @@ #include "flutter/fml/thread_local.h" -namespace fml { - #if FML_THREAD_LOCAL_PTHREADS -ThreadLocal::ThreadLocal() : ThreadLocal(nullptr) {} - -ThreadLocal::ThreadLocal(ThreadLocalDestroyCallback destroy) - : destroy_(destroy) { - auto callback = - reinterpret_cast(&ThreadLocal::ThreadLocalDestroy); - FML_CHECK(pthread_key_create(&_key, callback) == 0); -} - -ThreadLocal::~ThreadLocal() { - // This will NOT call the destroy callbacks on thread local values still - // active in other threads. Those must be cleared manually. The usage - // of this class should be similar to the thread_local keyword but with - // with a static storage specifier +#include "flutter/fml/logging.h" - // Collect the container - delete reinterpret_cast(pthread_getspecific(_key)); +namespace fml { +namespace internal { - // Finally, collect the key - FML_CHECK(pthread_key_delete(_key) == 0); +ThreadLocalPointer::ThreadLocalPointer(void (*destroy)(void*)) { + FML_CHECK(pthread_key_create(&key_, destroy) == 0); } -ThreadLocal::Box::Box(ThreadLocalDestroyCallback destroy, intptr_t value) - : destroy_(destroy), value_(value) {} - -ThreadLocal::Box::~Box() = default; - -#else // FML_THREAD_LOCAL_PTHREADS - -ThreadLocal::ThreadLocal() : ThreadLocal(nullptr) {} - -ThreadLocal::ThreadLocal(ThreadLocalDestroyCallback destroy) - : destroy_(destroy), value_(0) {} - -void ThreadLocal::Set(intptr_t value) { - if (value_ == value) { - return; - } - - if (value_ != 0 && destroy_) { - destroy_(value_); - } - - value_ = value; +ThreadLocalPointer::~ThreadLocalPointer() { + FML_CHECK(pthread_key_delete(key_) == 0); } -intptr_t ThreadLocal::Get() { - return value_; +void* ThreadLocalPointer::get() const { + return pthread_getspecific(key_); } -ThreadLocal::~ThreadLocal() { - if (value_ != 0 && destroy_) { - destroy_(value_); - } +void* ThreadLocalPointer::swap(void* ptr) { + void* old_ptr = get(); + FML_CHECK(pthread_setspecific(key_, ptr) == 0); + return old_ptr; } -#endif // FML_THREAD_LOCAL_PTHREADS - +} // namespace internal } // namespace fml + +#endif // FML_THREAD_LOCAL_PTHREADS diff --git a/fml/thread_local.h b/fml/thread_local.h index 4bf518244a6e9..de49cda324a04 100644 --- a/fml/thread_local.h +++ b/fml/thread_local.h @@ -2,13 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_FML_THREAD_LOCAL_H_ -#define FLUTTER_FML_THREAD_LOCAL_H_ +#ifndef FLUTTER_FML_THREAD_LOCAL_UNIQUE_PTR_H_ +#define FLUTTER_FML_THREAD_LOCAL_UNIQUE_PTR_H_ -#include +#include #include "flutter/fml/build_config.h" -#include "flutter/fml/logging.h" #include "flutter/fml/macros.h" #define FML_THREAD_LOCAL_PTHREADS OS_MACOSX || OS_LINUX || OS_ANDROID @@ -19,103 +18,60 @@ namespace fml { -using ThreadLocalDestroyCallback = std::function; - #if FML_THREAD_LOCAL_PTHREADS -// thread_local is unavailable and we have to resort to pthreads. - #define FML_THREAD_LOCAL static -class ThreadLocal { - private: - class Box { - public: - Box(ThreadLocalDestroyCallback destroy, intptr_t value); - - ~Box(); +namespace internal { - intptr_t Value() const { return value_; } - - void SetValue(intptr_t value) { - if (value == value_) { - return; - } - - DestroyValue(); - value_ = value; - } +class ThreadLocalPointer { + public: + ThreadLocalPointer(void (*destroy)(void*)); + ~ThreadLocalPointer(); - void DestroyValue() { - if (destroy_) { - destroy_(value_); - } - } + void* get() const; + void* swap(void* ptr); - private: - ThreadLocalDestroyCallback destroy_; - intptr_t value_; + private: + pthread_key_t key_; - FML_DISALLOW_COPY_AND_ASSIGN(Box); - }; + FML_DISALLOW_COPY_AND_ASSIGN(ThreadLocalPointer); +}; - static inline void ThreadLocalDestroy(void* value) { - FML_CHECK(value != nullptr); - auto* box = reinterpret_cast(value); - box->DestroyValue(); - delete box; - } +} // namespace internal +template +class ThreadLocalUniquePtr { public: - ThreadLocal(); - - ThreadLocal(ThreadLocalDestroyCallback destroy); - - void Set(intptr_t value) { - auto* box = reinterpret_cast(pthread_getspecific(_key)); - if (box == nullptr) { - box = new Box(destroy_, value); - FML_CHECK(pthread_setspecific(_key, box) == 0); - } else { - box->SetValue(value); - } - } - - intptr_t Get() { - auto* box = reinterpret_cast(pthread_getspecific(_key)); - return box != nullptr ? box->Value() : 0; - } + ThreadLocalUniquePtr() : ptr_(destroy) {} - ~ThreadLocal(); + T* get() const { return reinterpret_cast(ptr_.get()); } + void reset(T* ptr) { destroy(ptr_.swap(ptr)); } private: - pthread_key_t _key; - ThreadLocalDestroyCallback destroy_; + static void destroy(void* ptr) { delete reinterpret_cast(ptr); } - FML_DISALLOW_COPY_AND_ASSIGN(ThreadLocal); + internal::ThreadLocalPointer ptr_; + + FML_DISALLOW_COPY_AND_ASSIGN(ThreadLocalUniquePtr); }; #else // FML_THREAD_LOCAL_PTHREADS -#define FML_THREAD_LOCAL thread_local +#define FML_THREAD_LOCAL static thread_local -class ThreadLocal { +template +class ThreadLocalUniquePtr { public: - ThreadLocal(); - - ThreadLocal(ThreadLocalDestroyCallback destroy); - - void Set(intptr_t value); - - intptr_t Get(); + ThreadLocalUniquePtr() = default; - ~ThreadLocal(); + T* get() const { return ptr_.get(); } + void reset(T* ptr) { ptr_.reset(ptr); } private: - ThreadLocalDestroyCallback destroy_; - intptr_t value_; + std::unique_ptr ptr_; - FML_DISALLOW_COPY_AND_ASSIGN(ThreadLocal); + FML_DISALLOW_COPY_AND_ASSIGN(ThreadLocalUniquePtr); }; #endif // FML_THREAD_LOCAL_PTHREADS @@ -128,4 +84,4 @@ class ThreadLocal { } // namespace fml -#endif // FLUTTER_FML_THREAD_LOCAL_H_ +#endif // FLUTTER_FML_THREAD_LOCAL_UNIQUE_PTR_H_ diff --git a/fml/thread_local_unittests.cc b/fml/thread_local_unittests.cc index 7f48cfcd532f6..e0e026b3b76db 100644 --- a/fml/thread_local_unittests.cc +++ b/fml/thread_local_unittests.cc @@ -2,108 +2,103 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include -#include "flutter/fml/logging.h" +#include "flutter/fml/macros.h" #include "flutter/fml/thread_local.h" #include "gtest/gtest.h" -// We are only going to test the pthreads based thread local boxes. -#if FML_THREAD_LOCAL_PTHREADS +namespace { + +class Box { + public: + Box(int value, std::atomic_int* destroys = nullptr) + : value_(value), destroys_(destroys) {} + ~Box() { + if (destroys_) { + ++*destroys_; + } + } + + int value() const { return value_; } + + private: + int value_; + std::atomic_int* destroys_; + + FML_DISALLOW_COPY_AND_ASSIGN(Box); +}; + +FML_THREAD_LOCAL fml::ThreadLocalUniquePtr local; + +} // namespace TEST(ThreadLocal, SimpleInitialization) { std::thread thread([&] { - fml::ThreadLocal local; + ASSERT_EQ(local.get(), nullptr); auto value = 100; - local.Set(value); - ASSERT_EQ(local.Get(), value); + local.reset(new Box(value)); + ASSERT_EQ(local.get()->value(), value); }); thread.join(); } TEST(ThreadLocal, SimpleInitializationCheckInAnother) { std::thread thread([&] { - fml::ThreadLocal local; + ASSERT_EQ(local.get(), nullptr); auto value = 100; - local.Set(value); - ASSERT_EQ(local.Get(), value); - std::thread thread2([&]() { ASSERT_EQ(local.Get(), 0); }); + local.reset(new Box(value)); + ASSERT_EQ(local.get()->value(), value); + std::thread thread2([&]() { ASSERT_EQ(local.get(), nullptr); }); thread2.join(); }); thread.join(); } TEST(ThreadLocal, DestroyCallback) { + std::atomic_int destroys{0}; std::thread thread([&] { - int destroys = 0; - fml::ThreadLocal local([&destroys](intptr_t) { destroys++; }); + ASSERT_EQ(local.get(), nullptr); auto value = 100; - local.Set(value); - ASSERT_EQ(local.Get(), value); - ASSERT_EQ(destroys, 0); + local.reset(new Box(value, &destroys)); + ASSERT_EQ(local.get()->value(), value); + ASSERT_EQ(destroys.load(), 0); }); thread.join(); + ASSERT_EQ(destroys.load(), 1); } TEST(ThreadLocal, DestroyCallback2) { + std::atomic_int destroys{0}; std::thread thread([&] { - int destroys = 0; - fml::ThreadLocal local([&destroys](intptr_t) { destroys++; }); - - local.Set(100); - ASSERT_EQ(local.Get(), 100); - ASSERT_EQ(destroys, 0); - local.Set(200); - ASSERT_EQ(local.Get(), 200); - ASSERT_EQ(destroys, 1); + local.reset(new Box(100, &destroys)); + ASSERT_EQ(local.get()->value(), 100); + ASSERT_EQ(destroys.load(), 0); + local.reset(new Box(200, &destroys)); + ASSERT_EQ(local.get()->value(), 200); + ASSERT_EQ(destroys.load(), 1); }); thread.join(); + ASSERT_EQ(destroys.load(), 2); } TEST(ThreadLocal, DestroyThreadTimeline) { + std::atomic_int destroys{0}; std::thread thread([&] { - int destroys = 0; - fml::ThreadLocal local([&destroys](intptr_t) { destroys++; }); - - std::thread thread([&]() { - local.Set(100); - ASSERT_EQ(local.Get(), 100); - ASSERT_EQ(destroys, 0); - local.Set(200); - ASSERT_EQ(local.Get(), 200); - ASSERT_EQ(destroys, 1); + std::thread thread2([&]() { + local.reset(new Box(100, &destroys)); + ASSERT_EQ(local.get()->value(), 100); + ASSERT_EQ(destroys.load(), 0); + local.reset(new Box(200, &destroys)); + ASSERT_EQ(local.get()->value(), 200); + ASSERT_EQ(destroys.load(), 1); }); - ASSERT_EQ(local.Get(), 0); - thread.join(); - ASSERT_EQ(local.Get(), 0); - ASSERT_EQ(destroys, 2); - }); - thread.join(); -} - -TEST(ThreadLocal, SettingSameValue) { - std::thread thread([&] { - int destroys = 0; - { - fml::ThreadLocal local([&destroys](intptr_t) { destroys++; }); - - local.Set(100); - ASSERT_EQ(destroys, 0); - local.Set(100); - local.Set(100); - local.Set(100); - ASSERT_EQ(local.Get(), 100); - local.Set(100); - local.Set(100); - ASSERT_EQ(destroys, 0); - local.Set(200); - ASSERT_EQ(destroys, 1); - ASSERT_EQ(local.Get(), 200); - } - - ASSERT_EQ(destroys, 1); + ASSERT_EQ(local.get(), nullptr); + thread2.join(); + ASSERT_EQ(local.get(), nullptr); + ASSERT_EQ(destroys.load(), 2); }); thread.join(); + ASSERT_EQ(destroys.load(), 2); } - -#endif // FML_THREAD_LOCAL_PTHREADS diff --git a/lib/ui/compositing/scene_host.cc b/lib/ui/compositing/scene_host.cc index 84decb9a61f76..56aea7a61afdb 100644 --- a/lib/ui/compositing/scene_host.cc +++ b/lib/ui/compositing/scene_host.cc @@ -20,9 +20,8 @@ namespace { using SceneHostBindings = std::unordered_map; -FML_THREAD_LOCAL fml::ThreadLocal tls_scene_host_bindings([](intptr_t value) { - delete reinterpret_cast(value); -}); +FML_THREAD_LOCAL fml::ThreadLocalUniquePtr + tls_scene_host_bindings; void SceneHost_constructor(Dart_NativeArguments args) { tonic::DartCallConstructor(&flutter::SceneHost::Create, args); @@ -31,17 +30,15 @@ void SceneHost_constructor(Dart_NativeArguments args) { void SceneHost_constructorViewHolderToken(Dart_NativeArguments args) { // This UI thread / Isolate contains at least 1 SceneHost. Initialize the // per-Isolate bindings. - if (tls_scene_host_bindings.Get() == 0) { - tls_scene_host_bindings.Set( - reinterpret_cast(new SceneHostBindings())); + if (tls_scene_host_bindings.get() == nullptr) { + tls_scene_host_bindings.reset(new SceneHostBindings()); } tonic::DartCallConstructor(&flutter::SceneHost::CreateViewHolder, args); } flutter::SceneHost* GetSceneHost(scenic::ResourceId id) { - auto* bindings = - reinterpret_cast(tls_scene_host_bindings.Get()); + auto* bindings = tls_scene_host_bindings.get(); FML_DCHECK(bindings); auto binding = bindings->find(id); @@ -147,8 +144,7 @@ SceneHost::SceneHost(fml::RefPtr viewHolderTokenHandle, } auto bind_callback = [scene_host = this](scenic::ResourceId id) { - auto* bindings = - reinterpret_cast(tls_scene_host_bindings.Get()); + auto* bindings = tls_scene_host_bindings.get(); FML_DCHECK(bindings); FML_DCHECK(bindings->find(id) == bindings->end()); @@ -171,8 +167,7 @@ SceneHost::SceneHost(fml::RefPtr viewHolderTokenHandle, SceneHost::~SceneHost() { if (use_view_holder_) { - auto* bindings = - reinterpret_cast(tls_scene_host_bindings.Get()); + auto* bindings = tls_scene_host_bindings.get(); FML_DCHECK(bindings); bindings->erase(id_);