Skip to content

Commit f62afe6

Browse files
Asset ImageFetcher thread correctness in DEBUG builds (facebook#54487)
Summary: Changelog: [Internal] Reviewed By: lenaic Differential Revision: D86678354
1 parent b44e4fa commit f62afe6

File tree

2 files changed

+58
-6
lines changed

2 files changed

+58
-6
lines changed

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

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77

88
#include "ImageFetcher.h"
99

10+
#include <cxxreact/TraceSection.h>
11+
#include <glog/logging.h>
1012
#include <react/common/mapbuffer/JReadableMapBuffer.h>
13+
#include <react/debug/react_native_assert.h>
1114
#include <react/featureflags/ReactNativeFeatureFlags.h>
1215
#include <react/renderer/imagemanager/conversions.h>
1316

@@ -17,10 +20,19 @@ extern const char ImageFetcherKey[] = "ImageFetcher";
1720

1821
ImageFetcher::ImageFetcher(
1922
std::shared_ptr<const ContextContainer> contextContainer)
20-
: contextContainer_(std::move(contextContainer)) {}
23+
: contextContainer_(std::move(contextContainer))
24+
#ifdef REACT_NATIVE_DEBUG
25+
,
26+
threadId_(std::this_thread::get_id())
27+
#endif
28+
{
29+
}
2130

2231
ImageFetcher::~ImageFetcher() noexcept {
2332
if (ReactNativeFeatureFlags::enableImagePrefetchingJNIBatchingAndroid()) {
33+
#ifdef REACT_NATIVE_DEBUG
34+
assertThread();
35+
#endif
2436
contextContainer_->erase(ImageFetcherKey);
2537
}
2638
}
@@ -30,6 +42,7 @@ ImageRequest ImageFetcher::requestImage(
3042
SurfaceId surfaceId,
3143
const ImageRequestParams& imageRequestParams,
3244
Tag tag) {
45+
TraceSection s("ImageFetcher::requestImage");
3346
items_[surfaceId].emplace_back(
3447
ImageRequestItem{
3548
.imageSource = imageSource,
@@ -42,10 +55,24 @@ ImageRequest ImageFetcher::requestImage(
4255
flushImageRequests();
4356
}
4457

45-
return ImageRequest{imageSource, telemetry};
58+
auto sharedResumeFunction = SharedFunction<>();
59+
auto sharedCancelationFunction = SharedFunction<>();
60+
sharedResumeFunction.assign([]() {
61+
LOG(WARNING) << "SNIP:ImageFetcher::requestImage: resume callback";
62+
});
63+
sharedCancelationFunction.assign([]() {
64+
LOG(WARNING) << "SNIP:ImageFetcher::requestImage: cancel callback";
65+
});
66+
return ImageRequest{
67+
imageSource, telemetry, sharedResumeFunction, sharedCancelationFunction};
4668
}
4769

4870
void ImageFetcher::flushImageRequests() {
71+
#ifdef REACT_NATIVE_DEBUG
72+
if (ReactNativeFeatureFlags::enableImagePrefetchingJNIBatchingAndroid()) {
73+
assertThread();
74+
}
75+
#endif
4976
if (items_.empty()) {
5077
return;
5178
}
@@ -59,13 +86,30 @@ void ImageFetcher::flushImageRequests() {
5986
"experimental_prefetchResources");
6087

6188
for (auto& [surfaceId, surfaceImageRequests] : items_) {
62-
auto readableMapBuffer = JReadableMapBuffer::createWithContents(
63-
serializeImageRequests(surfaceImageRequests));
64-
prefetchResources(
65-
fabricUIManager_, surfaceId, "RCTImageView", readableMapBuffer.get());
89+
jni::local_ref<JReadableMapBuffer::jhybridobject> readableMapBuffer =
90+
nullptr;
91+
{
92+
TraceSection s("ImageFetcher::createWithContents");
93+
readableMapBuffer = JReadableMapBuffer::createWithContents(
94+
serializeImageRequests(surfaceImageRequests));
95+
}
96+
97+
{
98+
TraceSection s("ImageFetcher::experimental_prefetchResource");
99+
prefetchResources(
100+
fabricUIManager_, surfaceId, "RCTImageView", readableMapBuffer.get());
101+
}
66102
}
67103

68104
items_.clear();
69105
}
70106

107+
#ifdef REACT_NATIVE_DEBUG
108+
void ImageFetcher::assertThread() const {
109+
react_native_assert(
110+
threadId_ != std::this_thread::get_id() &&
111+
"ImageFetcher method called from the wrong thread!");
112+
}
113+
#endif
114+
71115
} // namespace facebook::react

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
#include <memory>
1414
#include <unordered_map>
1515
#include <vector>
16+
#ifdef REACT_NATIVE_DEBUG
17+
#include <thread>
18+
#endif
1619

1720
namespace facebook::react {
1821

@@ -39,5 +42,10 @@ class ImageFetcher {
3942

4043
std::unordered_map<SurfaceId, std::vector<ImageRequestItem>> items_;
4144
std::shared_ptr<const ContextContainer> contextContainer_;
45+
46+
#ifdef REACT_NATIVE_DEBUG
47+
std::thread::id threadId_;
48+
void assertThread() const;
49+
#endif
4250
};
4351
} // namespace facebook::react

0 commit comments

Comments
 (0)