Skip to content

Commit

Permalink
Avoid copying ReactPromise in ImageLoader module
Browse files Browse the repository at this point in the history
We're seeing crashes when the React instance is destroyed if there is an active ImageLoader::getSizeWithHeaders request in flight (related to JSI trying to create an object on a destroyed runtime).

This is similar to an issue that was resolved and picked into React Native Windows v0.68 (microsoft#10436), but even with that fix, the crash still occurs. I suspect it may be related to using the ReactPromise copy constructor, but even if not, it's not so bad to avoid copying the ReactPromise.
  • Loading branch information
rozele committed Mar 20, 2023
1 parent 3a24fbb commit 60bc5ce
Showing 1 changed file with 21 additions and 16 deletions.
37 changes: 21 additions & 16 deletions vnext/Microsoft.ReactNative/Modules/ImageViewManagerModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,15 @@ winrt::com_ptr<IWICBitmapSource> wicBitmapSourceFromStream(
winrt::fire_and_forget GetImageSizeAsync(
std::string uriString,
winrt::Microsoft::ReactNative::JSValue &&headers,
Mso::Functor<void(int32_t width, int32_t height)> successCallback,
Mso::Functor<void()> errorCallback
Mso::Functor<void(int32_t width, int32_t height, bool succeeded)> callback,
#ifdef USE_FABRIC
,
bool useFabric
#endif // USE_FABRIC
) {
bool succeeded{false};
int32_t width{};
int32_t height{};

try {
ReactImageSource source;
Expand Down Expand Up @@ -75,24 +76,25 @@ winrt::fire_and_forget GetImageSizeAsync(
co_await bitmap.SetSourceAsync(memoryStream);
}
if (bitmap) {
successCallback(bitmap.PixelWidth(), bitmap.PixelHeight());
width = bitmap.PixelWidth();
height = bitmap.PixelHeight();
succeeded = true;
}
#ifdef USE_FABRIC
} else {
UINT width, height;
UINT bmpWidth, bmpHeight;
auto wicBmpSource = wicBitmapSourceFromStream(memoryStream);
if (SUCCEEDED(wicBmpSource->GetSize(&width, &height))) {
successCallback(width, height);
if (SUCCEEDED(wicBmpSource->GetSize(&bmpWidth, &bmpHeight))) {
width = static_cast<int32_t>(bmpWidth);
height = static_cast<int32_t>(bmpHeight);
succeeded = true;
}
}
#endif // USE_FABRIC
} catch (winrt::hresult_error const &) {
}

if (!succeeded)
errorCallback();
callback(width, height, succeeded);

co_return;
}
Expand Down Expand Up @@ -135,14 +137,17 @@ void ImageLoader::getSizeWithHeaders(
GetImageSizeAsync(
std::move(uri),
std::move(headers),
[result, context](double width, double height) noexcept {
context.JSDispatcher().Post([result = std::move(result), width, height]() noexcept {
result.Resolve(
Microsoft::ReactNativeSpecs::ImageLoaderIOSSpec_getSizeWithHeaders_returnType{width, height});
});
},
[result, context]() noexcept {
context.JSDispatcher().Post([result = std::move(result)]() noexcept { result.Reject("Failed"); });
[result = std::move(result), context](double width, double height, bool succeeded) noexcept {
if (succeeded) {
context.JSDispatcher().Post([result = std::move(result), width, height]() noexcept {
result.Resolve(
Microsoft::ReactNativeSpecs::ImageLoaderIOSSpec_getSizeWithHeaders_returnType{width, height});
});
} else {
context.JSDispatcher().Post([result = std::move(result)]() noexcept {
result.Reject("Failed");
});
}
}
#ifdef USE_FABRIC
,
Expand Down

0 comments on commit 60bc5ce

Please sign in to comment.