diff --git a/packages/react-native/ReactCommon/react/utils/SimpleThreadSafeCache.h b/packages/react-native/ReactCommon/react/utils/SimpleThreadSafeCache.h index 3cc22f7fbcac..3624359f0d67 100644 --- a/packages/react-native/ReactCommon/react/utils/SimpleThreadSafeCache.h +++ b/packages/react-native/ReactCommon/react/utils/SimpleThreadSafeCache.h @@ -79,15 +79,27 @@ class SimpleThreadSafeCache { auto getMapIterator(const KeyT &key, CacheGeneratorFunction auto generator) const { - std::lock_guard lock(mutex_); + { + std::lock_guard lock(mutex_); + if (auto it = map_.find(key); it != map_.end()) { + // Move accessed item to front of list + list_.splice(list_.begin(), list_, it->second); + return it; + } + } + + // The generator runs without the lock: it may block on another thread + // that needs this cache. On iOS, text measurement creates an + // NSTextStorage whose editing notification can wait for the main queue, + // while the main thread waits for this lock to measure text. + auto value = generator(); + std::lock_guard lock(mutex_); if (auto it = map_.find(key); it != map_.end()) { - // Move accessed item to front of list + // Another thread generated the same key meanwhile list_.splice(list_.begin(), list_, it->second); return it; } - - auto value = generator(); // Add new value to front of list and map list_.emplace_front(key, value); auto [it, _] = map_.insert_or_assign(key, list_.begin()); diff --git a/packages/react-native/ReactCommon/react/utils/tests/SimpleThreadSafeCacheTest.cpp b/packages/react-native/ReactCommon/react/utils/tests/SimpleThreadSafeCacheTest.cpp index 1fc5812769fa..c6a07701f5c8 100644 --- a/packages/react-native/ReactCommon/react/utils/tests/SimpleThreadSafeCacheTest.cpp +++ b/packages/react-native/ReactCommon/react/utils/tests/SimpleThreadSafeCacheTest.cpp @@ -8,6 +8,10 @@ #include #include +#include +#include +#include + namespace facebook::react { TEST(EvictingCacheMapTest, BasicInsertAndGet) { @@ -37,4 +41,40 @@ TEST(EvictingCacheMapTest, Eviction) { EXPECT_EQ(cache.get(3), "three"); } +TEST(EvictingCacheMapTest, GeneratorDoesNotBlockOtherThreads) { + SimpleThreadSafeCache cache; + std::promise otherThreadDone; + std::thread other; + + // The generator waits for a second thread that reads the same cache, as an + // iOS text measurement waits for the main thread. + auto value = cache.get(1, [&]() { + other = std::thread([&]() { + cache.get(2, []() { return std::string("two"); }); + otherThreadDone.set_value(); + }); + auto status = + otherThreadDone.get_future().wait_for(std::chrono::seconds(5)); + return std::string( + status == std::future_status::ready ? "one" : "blocked"); + }); + other.join(); + + EXPECT_EQ(value, "one"); + EXPECT_EQ(cache.get(2), "two"); +} + +TEST(EvictingCacheMapTest, ConcurrentMissKeepsFirstInsertedValue) { + SimpleThreadSafeCache cache; + + auto value = cache.get(1, [&]() { + std::thread other([&]() { cache.get(1, []() { return std::string("first"); }); }); + other.join(); + return std::string("second"); + }); + + EXPECT_EQ(value, "first"); + EXPECT_EQ(cache.get(1), "first"); +} + } // namespace facebook::react