Skip to content

Commit 1d452b1

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Release cached images when image component gets recycled on iOS (#51493)
Summary: Pull Request resolved: #51493 Changelog: [IOS][FIXED] Don't retain cached images in state after `RCTImageComponentView` gets recycled Fixes #51198 Crosspost from the task comment: From what I've been able to figure out, it seems like the image shadow nodes (keeping the loaded image in state) are being kept in memory by shadow node reference wrappers. It doesn't seem strictly like a memory leak - manually triggering garbage collection causes those nodes to be deallocated, but since Hermes isn't aware of the memory they are retaining, I think, it doesn't trigger it automatically. This diff releases the image data when the observers are notified and adds a new (`Consumed`) status to signify that. Reviewed By: sammy-SC Differential Revision: D75137263 fbshipit-source-id: 97eda7e6d1ef5cd633c4a5a4c37babc5e08968fb
1 parent 6747f2b commit 1d452b1

3 files changed

Lines changed: 21 additions & 1 deletion

File tree

packages/react-native/ReactCommon/react/renderer/imagemanager/ImageResponse.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class ImageResponse final {
2121
Completed,
2222
Failed,
2323
Cancelled,
24+
Consumed,
2425
};
2526

2627
ImageResponse(std::shared_ptr<void> image, std::shared_ptr<void> metadata);

packages/react-native/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <algorithm>
1111

1212
#include <react/debug/react_native_assert.h>
13+
#include <react/featureflags/ReactNativeFeatureFlags.h>
1314

1415
namespace facebook::react {
1516

@@ -31,6 +32,7 @@ void ImageResponseObserverCoordinator::addObserver(
3132
case ImageResponse::Status::Completed: {
3233
auto imageData = imageData_;
3334
auto imageMetadata = imageMetadata_;
35+
consumeResponse();
3436
mutex_.unlock();
3537
observer.didReceiveImage(ImageResponse{imageData, imageMetadata});
3638
break;
@@ -41,7 +43,8 @@ void ImageResponseObserverCoordinator::addObserver(
4143
observer.didReceiveFailure(ImageLoadError{imageErrorData});
4244
break;
4345
}
44-
case ImageResponse::Status::Cancelled: {
46+
case ImageResponse::Status::Cancelled:
47+
case ImageResponse::Status::Consumed: {
4548
observers_.push_back(&observer);
4649
status_ = ImageResponse::Status::Loading;
4750
mutex_.unlock();
@@ -93,6 +96,9 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete(
9396
status_ == ImageResponse::Status::Cancelled);
9497
status_ = ImageResponse::Status::Completed;
9598
auto observers = observers_;
99+
if (!observers.empty()) {
100+
consumeResponse();
101+
}
96102
mutex_.unlock();
97103

98104
for (auto observer : observers) {
@@ -116,4 +122,12 @@ void ImageResponseObserverCoordinator::nativeImageResponseFailed(
116122
}
117123
}
118124

125+
void ImageResponseObserverCoordinator::consumeResponse() const {
126+
if (ReactNativeFeatureFlags::releaseImageDataWhenConsumed()) {
127+
status_ = ImageResponse::Status::Consumed;
128+
imageData_.reset();
129+
imageMetadata_.reset();
130+
}
131+
}
132+
119133
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ class ImageResponseObserverCoordinator {
6161
void nativeImageResponseFailed(const ImageLoadError& loadError) const;
6262

6363
private:
64+
/*
65+
* Resets the cached image data pointers. Needs to be protected by mutex_.
66+
*/
67+
void consumeResponse() const;
68+
6469
/*
6570
* List of observers.
6671
* Mutable: protected by mutex_.

0 commit comments

Comments
 (0)