Skip to content

Commit 0fabe26

Browse files
Nick Lefevermeta-codesync[bot]
authored andcommitted
Make image prefetch JNI batching thread-safe (#54861)
Summary: Pull Request resolved: #54861 The JNI batching for image prefetching was leading to crashes due to the following thread-safety issue in the ImageFetcher implementation: * Image requests are made from the ImageShadowNode layout call, these can happen from any thread. * Image request flushing happens from `FabricUIManagerBinding::schedulerShouldRenderTransactions()`, meaning on mount * A commit could happen while a mount is running. Meaning the image fetcher `items_` read/write has to be synchronized to avoid data corruption. Changelog: [Internal] Reviewed By: javache Differential Revision: D88970631 fbshipit-source-id: ef25e3b78e6c23a786c8e225eb3abf0bc04213a5
1 parent 41380f3 commit 0fabe26

2 files changed

Lines changed: 18 additions & 10 deletions

File tree

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/android/react/renderer/imagemanager/ImageFetcher.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,14 @@ ImageRequest ImageFetcher::requestImage(
2424
SurfaceId surfaceId,
2525
const ImageRequestParams& imageRequestParams,
2626
Tag tag) {
27-
items_[surfaceId].emplace_back(
28-
ImageRequestItem{
29-
.imageSource = imageSource,
30-
.imageRequestParams = imageRequestParams,
31-
.tag = tag});
27+
{
28+
std::lock_guard<std::mutex> lock(mutex_);
29+
items_[surfaceId].emplace_back(
30+
ImageRequestItem{
31+
.imageSource = imageSource,
32+
.imageRequestParams = imageRequestParams,
33+
.tag = tag});
34+
}
3235

3336
auto telemetry = std::make_shared<ImageTelemetry>(surfaceId);
3437

@@ -40,8 +43,13 @@ ImageRequest ImageFetcher::requestImage(
4043
}
4144

4245
void ImageFetcher::flushImageRequests() {
43-
if (items_.empty()) {
44-
return;
46+
std::unordered_map<SurfaceId, std::vector<ImageRequestItem>> items;
47+
{
48+
std::lock_guard<std::mutex> lock(mutex_);
49+
if (items_.empty()) {
50+
return;
51+
}
52+
items.swap(items_);
4553
}
4654

4755
auto fabricUIManager_ =
@@ -52,14 +60,12 @@ void ImageFetcher::flushImageRequests() {
5260
SurfaceId, std::string, JReadableMapBuffer::javaobject)>(
5361
"experimental_prefetchResources");
5462

55-
for (auto& [surfaceId, surfaceImageRequests] : items_) {
63+
for (auto& [surfaceId, surfaceImageRequests] : items) {
5664
auto readableMapBuffer = JReadableMapBuffer::createWithContents(
5765
serializeImageRequests(surfaceImageRequests));
5866
prefetchResources(
5967
fabricUIManager_, surfaceId, "RCTImageView", readableMapBuffer.get());
6068
}
61-
62-
items_.clear();
6369
}
6470

6571
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/android/react/renderer/imagemanager/ImageFetcher.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <react/renderer/imagemanager/ImageRequestParams.h>
1212
#include <react/utils/ContextContainer.h>
1313
#include <memory>
14+
#include <mutex>
1415
#include <unordered_map>
1516
#include <vector>
1617

@@ -38,6 +39,7 @@ class ImageFetcher {
3839
Tag tag);
3940

4041
std::unordered_map<SurfaceId, std::vector<ImageRequestItem>> items_;
42+
std::mutex mutex_;
4143
std::shared_ptr<const ContextContainer> contextContainer_;
4244
};
4345
} // namespace facebook::react

0 commit comments

Comments
 (0)