Skip to content

Fix use-after-free when an Image is destroyed mid-download - #16345

Open
Gordon MacMaster (gmacmaster) wants to merge 1 commit into
microsoft:mainfrom
Virtual-Fulfillment-Technologies-Inc:vendora/fix-image-free-after-use
Open

Fix use-after-free when an Image is destroyed mid-download#16345
Gordon MacMaster (gmacmaster) wants to merge 1 commit into
microsoft:mainfrom
Virtual-Fulfillment-Technologies-Inc:vendora/fix-image-free-after-use

Conversation

@gmacmaster

@gmacmaster Gordon MacMaster (gmacmaster) commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Why

ImageResponseObserverCoordinator (in react-native core) copies its observer list under a lock, releases the lock, and then dereferences the raw observer pointers:

mutex_.lock();
auto observers = observers_;
mutex_.unlock();

for (auto observer : observers) {
  observer->didReceiveProgress(progress, loaded, total);
}

WindowsImageManager notifies that coordinator from the download thread and from the IAsyncOperation completion handler. Observers, however, are added and removed on the UI thread (ImageComponentView::setStateAndResubscribeImageResponseObserver), and the UI thread is also where the owning ImageComponentView — and with it the std::shared_ptr<WindowsImageResponseObserver> — is destroyed.

So if a view is torn down while its download is still streaming, the observer can be freed between the snapshot and the dereference. The first thing WindowsImageResponseObserver::didReceiveProgress touches is its winrt::weak_ref member, copied into the dispatcher lambda, so the crash surfaces as an EXCEPTION_ACCESS_VIOLATION_READ inside winrt::com_ptr<T>::add_ref:

at Microsoft::ReactNative::WindowsImageManager::GetImageRandomAccessStreamAsync$_ResumeCoro$1 (WindowsImageManager.cpp:172)
at Microsoft::ReactNative::WindowsImageManager::requestImage::__l7::<T>::operator() (WindowsImageManager.cpp:208)
at facebook::react::ImageResponseObserverCoordinator::nativeImageResponseProgress (ImageResponseObserverCoordinator.cpp:85)
at ...ImageComponentView::WindowsImageResponseObserver::didReceiveProgress (ImageComponentView.cpp:40)
at winrt::com_ptr<T>::{ctor} (base.h:2385)
at winrt::com_ptr<T>::add_ref (base.h:2561)

The weak_ref doesn't help here — it guards the ImageComponentView, but the object that was freed is the observer that holds it.

This is the Fabric recurrence of a lifetime bug that was fixed in the Paper ReactImage back in #2820 and #2822. The scenario described there is the same one that reproduces today:

Scenario wise we are hitting this when going to a screen with a bunch of images being downloaded, if you navigate away so all those Image elements are destructed, we crash in a weakref dtor with invalid heap free stuff happening

The Fabric completion handler has always notified the coordinator off the UI thread, but the window was narrow because it fired once per request. #14493 ([Fabric] Implement onProgress for Image) added a progressCallback that calls nativeImageResponseProgress from the download loop on every 16 KB chunk, which widened that window enormously and is what makes this reachable in practice. #13440 is where the observer gained its winrt::weak_ref, which resolved an unrelated reference cycle but does not protect against the observer itself being freed.

We hit this in production on RNW 0.84 — a screen of remote thumbnails, navigated away from mid-load.

Resolves

N/A

What

All coordinator notifications are now marshalled onto the UI thread, so notification and subscription are serialized on the same thread and the coordinator can no longer iterate a snapshot while another thread frees an entry. Changes are confined to requestImage in vnext/Microsoft.ReactNative/Fabric/WindowsImageManager.cpp:

  • progressCallback computes the progress fraction on the download thread and posts only the nativeImageResponseProgress call.
  • The Completed handler keeps ResolveImage() on the completion thread, so WIC decoding stays off the UI thread, and posts only the nativeImageResponseComplete / nativeImageResponseFailed call via two small postComplete / postFailure helpers.
  • The weak coordinator pointer is locked at delivery time rather than at call time, which is the correct liveness check now that notification is deferred.

This incidentally moves didReceiveFailure onto the UI thread as well. It previously resolved the weak ref and touched ImageComponentView state directly on the calling thread, which was a separate off-thread access to view state.

Note for reviewers: the inner UIDispatcher().Post calls in ImageComponentView::didReceiveProgress and didReceiveImage are now a redundant second hop. They're harmless and FIFO ordering through the serial dispatcher is preserved, so I left them out of this change to keep the diff to a single file. Happy to fold the cleanup in if you'd prefer.

Screenshots

N/A — crash fix with no visual change.

Testing

No automated tests added. The failure is a data race on object lifetime, so a deterministic unit test would require injecting a teardown between the coordinator's snapshot and its dereference, which isn't reachable from RNW without changing core.

Changelog

Yes.

Fixed a crash that could occur when an Image was destroyed while its download was still in flight, such as navigating away from a screen before its images finished loading.

Microsoft Reviewers: Open in CodeFlow

@gmacmaster
Gordon MacMaster (gmacmaster) requested a review from a team as a code owner July 29, 2026 14:01
@azure-pipelines

Copy link
Copy Markdown
Contributor
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@vmoroz
Vladimir Morozov (vmoroz) enabled auto-merge (squash) August 3, 2026 16:37
@vmoroz

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Contributor
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Fabric image lifetime race where ImageResponseObserverCoordinator could dereference freed observer pointers when an Image is torn down mid-download. The change ensures observer notifications are delivered on the UI thread to serialize notification with observer subscription/unsubscription and view teardown.

Changes:

  • Marshal nativeImageResponseProgress, nativeImageResponseComplete, and nativeImageResponseFailed calls onto the UI thread from both the download loop and async completion handler.
  • Keep image decoding (ResolveImage()) off the UI thread while deferring only the coordinator notification.
  • Add a change file for release notes/versioning.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
vnext/Microsoft.ReactNative/Fabric/WindowsImageManager.cpp Posts coordinator progress/complete/failure notifications to the UI dispatcher to avoid use-after-free during teardown.
change/react-native-windows-eba3d592-2ea0-4067-8880-2beea6638837.json Records the crash fix for release/change tracking.

Comment on lines +215 to 222
auto progressCallback = [weakObserverCoordinator, uiDispatcher](int64_t loaded, int64_t total) {
float progress = total > 0 ? static_cast<float>(loaded) / static_cast<float>(total) : 1.0f;
uiDispatcher.Post([weakObserverCoordinator, progress, loaded, total]() {
if (auto observerCoordinator = weakObserverCoordinator.lock()) {
observerCoordinator->nativeImageResponseProgress(progress, loaded, total);
}
});
};
Comment on lines +217 to +221
uiDispatcher.Post([weakObserverCoordinator, progress, loaded, total]() {
if (auto observerCoordinator = weakObserverCoordinator.lock()) {
observerCoordinator->nativeImageResponseProgress(progress, loaded, total);
}
});
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants