From 1452954c4c5d15b04bc80fad359b2cf4f44addb3 Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Fri, 27 Sep 2019 13:42:45 -0700 Subject: [PATCH] TM: Add mutex to access LongLivedObjectCollection - making it thread safe Summary: There are cases where the CallbackWrapper instances were added from different thread, potentially crashing the inner std::unordered_set<> we're using to keep the wrappers alive for extended time. To avoid it, let's just use std::mutex. Reviewed By: shergin Differential Revision: D17631233 fbshipit-source-id: e8f98004e45a68be31f8f0cda118fb67dcb06d45 --- ReactCommon/turbomodule/core/LongLivedObject.cpp | 9 ++++++--- ReactCommon/turbomodule/core/LongLivedObject.h | 12 +++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/ReactCommon/turbomodule/core/LongLivedObject.cpp b/ReactCommon/turbomodule/core/LongLivedObject.cpp index a56ddd11125..6598bb2d19c 100644 --- a/ReactCommon/turbomodule/core/LongLivedObject.cpp +++ b/ReactCommon/turbomodule/core/LongLivedObject.cpp @@ -18,11 +18,13 @@ LongLivedObjectCollection &LongLivedObjectCollection::get() { LongLivedObjectCollection::LongLivedObjectCollection() {} -void LongLivedObjectCollection::add(std::shared_ptr so) { +void LongLivedObjectCollection::add(std::shared_ptr so) const { + std::lock_guard lock(collectionMutex_); collection_.insert(so); } -void LongLivedObjectCollection::remove(const LongLivedObject *o) { +void LongLivedObjectCollection::remove(const LongLivedObject *o) const { + std::lock_guard lock(collectionMutex_); auto p = collection_.begin(); for (; p != collection_.end(); p++) { if (p->get() == o) { @@ -34,7 +36,8 @@ void LongLivedObjectCollection::remove(const LongLivedObject *o) { } } -void LongLivedObjectCollection::clear() { +void LongLivedObjectCollection::clear() const { + std::lock_guard lock(collectionMutex_); collection_.clear(); } diff --git a/ReactCommon/turbomodule/core/LongLivedObject.h b/ReactCommon/turbomodule/core/LongLivedObject.h index f390029a568..ee5039b52da 100644 --- a/ReactCommon/turbomodule/core/LongLivedObject.h +++ b/ReactCommon/turbomodule/core/LongLivedObject.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include namespace facebook { @@ -32,7 +33,7 @@ class LongLivedObject { }; /** - * A singleton collection for the `LongLivedObject`s. + * A singleton, thread-safe, write-only collection for the `LongLivedObject`s. */ class LongLivedObjectCollection { public: @@ -41,13 +42,14 @@ class LongLivedObjectCollection { LongLivedObjectCollection(LongLivedObjectCollection const &) = delete; void operator=(LongLivedObjectCollection const &) = delete; - void add(std::shared_ptr o); - void remove(const LongLivedObject *o); - void clear(); + void add(std::shared_ptr o) const; + void remove(const LongLivedObject *o) const; + void clear() const; private: LongLivedObjectCollection(); - std::unordered_set> collection_; + mutable std::unordered_set> collection_; + mutable std::mutex collectionMutex_; }; } // namespace react