Skip to content

Commit 53bf98b

Browse files
mattijsfmeta-codesync[bot]
authored andcommitted
Ignore stale image callbacks after an Image view is recycled on iOS (#58669)
Summary: Fixes #58667. On iOS with the new architecture, an `<Image>` can show the image of another `<Image>` that was unmounted just before. `RCTImageResponseObserverProxy` dispatches its callbacks to the main queue. `RCTImageComponentView` used one proxy for its whole lifetime, and `didReceiveImage` only checked that the view still had a state. When a view was recycled and handed to a new `<Image>` while a callback for the old request was still queued, that callback was applied to the new image. If the new source was cached it had already been applied synchronously on subscribe, so the late old image stayed on screen. This change: - creates a new `RCTImageResponseObserverProxy` for every subscription in `_setStateAndResubscribeImageResponseObserver` - ignores image, progress and failure callbacks whose `observer` is not the current proxy (the `fromObserver:` argument was already passed but not used). For failures this also stops a stale error from clearing the new image. The proxy's address identifies the subscription. The new proxy is allocated before the previous one is released, so two consecutive subscriptions never share an address. Reuse of an old address would need two resubscriptions while a callback from the first one is still queued. ## Changelog: [IOS] [FIXED] - Image no longer shows the image of a previous source after its native view is recycled Pull Request resolved: #58669 Test Plan: Reproducer: https://github.com/mattijsf/rn-image-recycled-view-stale-image (React Native 0.87.1, also as a Snack: https://snack.expo.dev/mattijsf/image-recycle-stale-load-ios?platform=ios). It mounts 64 uncached remote images (red), replaces them with a cached one (green) as soon as the first red one has loaded, and counts a wrong image when the new image's `onLoad` reports the old image's size. A run is 50 rounds. Release build on the iOS simulator (iPhone 17 Pro Max, iOS 26.5), React Native built from source (`RCT_USE_PREBUILT_RNCORE=0`), one run of 50 rounds each. The diff applies unchanged to 0.87.1. | | rounds with a wrong image | wrong images | | --- | --- | --- | | 0.87.1 | 27 of 50 | 120 | | 0.87.1 with this change | 0 of 50 | 0 | With the change the red images still show up briefly before each swap, as intended, but none of them end up on a green tile. Reviewed By: christophpurrer Differential Revision: D121619393 Pulled By: javache fbshipit-source-id: 78c29450bb25083667515c95f8c704d3b7db5eef
1 parent 86c2cf1 commit 53bf98b

1 file changed

Lines changed: 13 additions & 5 deletions

File tree

‎packages/react-native/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ - (instancetype)initWithFrame:(CGRect)frame
3838
_imageView.layer.minificationFilter = kCAFilterTrilinear;
3939
_imageView.layer.magnificationFilter = kCAFilterTrilinear;
4040

41-
_imageResponseObserverProxy = std::make_shared<RCTImageResponseObserverProxy>(self);
42-
4341
self.contentView = _imageView;
4442
}
4543

@@ -124,6 +122,11 @@ - (void)_setStateAndResubscribeImageResponseObserver:(const ImageShadowNode::Con
124122
_state = state;
125123

126124
if (_state) {
125+
// A new observer per subscription: callbacks of a previous request can still be queued on the
126+
// main queue (e.g. after this view was recycled and reused), and must not be applied here.
127+
// The callbacks are matched by the proxy's address. The new proxy is allocated before the
128+
// previous one is released, so two consecutive subscriptions never share an address.
129+
_imageResponseObserverProxy = std::make_shared<RCTImageResponseObserverProxy>(self);
127130
auto &observerCoordinator = _state->getData().getImageRequest().getObserverCoordinator();
128131
observerCoordinator.addObserver(_imageResponseObserverProxy);
129132
}
@@ -140,8 +143,9 @@ - (void)prepareForRecycle
140143

141144
- (void)didReceiveImage:(UIImage *)image metadata:(id)metadata fromObserver:(const void *)observer
142145
{
143-
if (!_eventEmitter || !_state) {
144-
// Notifications are delivered asynchronously and might arrive after the view is already recycled.
146+
if (!_eventEmitter || !_state || observer != _imageResponseObserverProxy.get()) {
147+
// Notifications are delivered asynchronously and might arrive after the view is already recycled,
148+
// or after it has been reused for another image.
145149
// In the future, we should incorporate an `EventEmitter` into a separate object owned by `ImageRequest` or `State`.
146150
// See for more info: T46311063.
147151
return;
@@ -187,7 +191,7 @@ - (void)didReceiveProgress:(float)progress
187191
total:(int64_t)total
188192
fromObserver:(const void *)observer
189193
{
190-
if (!_eventEmitter) {
194+
if (!_eventEmitter || observer != _imageResponseObserverProxy.get()) {
191195
return;
192196
}
193197

@@ -196,6 +200,10 @@ - (void)didReceiveProgress:(float)progress
196200

197201
- (void)didReceiveFailure:(NSError *)error fromObserver:(const void *)observer
198202
{
203+
if (observer != _imageResponseObserverProxy.get()) {
204+
return;
205+
}
206+
199207
_imageView.image = nil;
200208

201209
if (!_eventEmitter) {

0 commit comments

Comments
 (0)