Skip to content

Commit

Permalink
Fabric: Fixes image coordinator status assert crash in debug mode (#4…
Browse files Browse the repository at this point in the history
…7655)

Summary:
Because we cancel the request and the callback for the request loading state occurs in a multi-threaded environment, we may still receive the callback even after canceling the request.

 The assert failed like below:
```
(lldb) bt
* thread #48, queue = 'com.apple.root.default-qos', stop reason = signal SIGABRT
    frame #0: 0x0000000106d15008 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x000000010581f408 libsystem_pthread.dylib`pthread_kill + 256
    frame #2: 0x000000018016c4ec libsystem_c.dylib`abort + 104
    frame #3: 0x000000018016b934 libsystem_c.dylib`__assert_rtn + 268
  * frame #4: 0x00000001073fcd64 React_Fabric`facebook::react::ImageResponseObserverCoordinator::nativeImageResponseComplete(this=0x00006000039bc838, imageResponse=0x000000016ce86970) const at ImageResponseObserverCoordinator.cpp:93:3
    frame #5: 0x00000001057c9a6c React_ImageManager`__42-[RCTImageManager requestImage:surfaceId:]_block_invoke_2(.block_descriptor=0x0000600000da21c0, error=0x0000000000000000, image=0x0000600003034ea0, metadata=0x0000000000000000) at RCTImageManager.mm:76:30
    frame #6: 0x0000000105657188 RCTImage`__140-[RCTImageLoader loadImageWithURLRequest:size:scale:clipped:resizeMode:priority:attribution:progressBlock:partialLoadBlock:completionBlock:]_block_invoke_3(.block_descriptor=0x0000600002c90200, error_=0x0000000000000000, image=0x0000600003034ea0) at RCTImageLoader.mm:831:11
    frame #7: 0x00000001056582e8 RCTImage`__80-[RCTImageLoader decodeImageData:size:scale:clipped:resizeMode:completionBlock:]_block_invoke_2(.block_descriptor=0x000060000267b780, error=0x0000000000000000, image=0x0000600003034ea0) at RCTImageLoader.mm:933:7
    frame #8: 0x0000000105658d60 RCTImage`__80-[RCTImageLoader decodeImageData:size:scale:clipped:resizeMode:completionBlock:]_block_invoke_3.207(.block_descriptor=0x0000600002957e90) at RCTImageLoader.mm:973:13
    frame #9: 0x0000000108560ec0 libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #10: 0x00000001085627b8 libdispatch.dylib`_dispatch_client_callout + 16
    frame #11: 0x00000001085655f4 libdispatch.dylib`_dispatch_queue_override_invoke + 1312
    frame #12: 0x00000001085763d4 libdispatch.dylib`_dispatch_root_queue_drain + 372
    frame #13: 0x0000000108576f7c libdispatch.dylib`_dispatch_worker_thread2 + 256
    frame #14: 0x000000010581bb38 libsystem_pthread.dylib`_pthread_wqthread + 224
```

![image](https://github.com/user-attachments/assets/7ed1997d-9d48-4631-9a02-0eef28a03cf3)

```
(lldb) bt
* thread #18, queue = 'com.meta.react.turbomodulemanager.queue', stop reason = signal SIGABRT
    frame #0: 0x0000000107ea9008 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x000000010569b408 libsystem_pthread.dylib`pthread_kill + 256
    frame #2: 0x000000018016c4ec libsystem_c.dylib`abort + 104
    frame #3: 0x000000018016b934 libsystem_c.dylib`__assert_rtn + 268
  * frame #4: 0x00000001072bcae8 React_Fabric`facebook::react::ImageResponseObserverCoordinator::nativeImageResponseProgress(this=0x00006000039bd7a8, progress=-1006, loaded=1006, total=-1) const at ImageResponseObserverCoordinator.cpp:76:3
    frame #5: 0x0000000105615fb8 React_ImageManager`__42-[RCTImageManager requestImage:surfaceId:]_block_invoke.3(.block_descriptor=0x0000600000d26a30, progress=1006, total=-1) at RCTImageManager.mm:89:28
    frame #6: 0x00000001055160fc RCTImage`__64-[RCTImageLoader _loadURLRequest:progressBlock:completionBlock:]_block_invoke.192(.block_descriptor=0x0000600000cd4900, progress=1006, total=-1) at RCTImageLoader.mm:747:7
    frame #7: 0x000000010504bc4c RCTNetwork`__44-[RCTNetworkTask URLRequest:didReceiveData:]_block_invoke.22(.block_descriptor=0x00006000017e0c80) at RCTNetworkTask.mm:201:7
    frame #8: 0x0000000108a24ec0 libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #9: 0x0000000108a267b8 libdispatch.dylib`_dispatch_client_callout + 16
    frame #10: 0x0000000108a2eaac libdispatch.dylib`_dispatch_lane_serial_drain + 912
    frame #11: 0x0000000108a2f7b0 libdispatch.dylib`_dispatch_lane_invoke + 420
    frame #12: 0x0000000108a3c1f0 libdispatch.dylib`_dispatch_root_queue_drain_deferred_wlh + 324
    frame #13: 0x0000000108a3b75c libdispatch.dylib`_dispatch_workloop_worker_thread + 732
    frame #14: 0x0000000105697b74 libsystem_pthread.dylib`_pthread_wqthread + 284
```

![image](https://github.com/user-attachments/assets/524a3ba2-857e-4f3a-8757-115d3ebadb42)

## Changelog:

[IOS] [FIXED] - Fabric: Fixes image coordinator status assert crash in debug mode

Pull Request resolved: #47655

Test Plan: Very easy to repro, just open RNTester's Image example, and then pop the page. Do this back and forth a few times.

Reviewed By: sammy-SC, rshest

Differential Revision: D66093834

Pulled By: javache

fbshipit-source-id: a2ca36147498725d38a95cc3fcc12a2c18802303
  • Loading branch information
zhongwuzw authored and facebook-github-bot committed Nov 18, 2024
1 parent 089c87e commit e0df58d
Showing 1 changed file with 9 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ void ImageResponseObserverCoordinator::nativeImageResponseProgress(
int64_t total) const {
mutex_.lock();
auto observers = observers_;
react_native_assert(status_ == ImageResponse::Status::Loading);
react_native_assert(
status_ == ImageResponse::Status::Loading ||
status_ == ImageResponse::Status::Cancelled);
mutex_.unlock();

for (auto observer : observers) {
Expand All @@ -86,7 +88,9 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete(
mutex_.lock();
imageData_ = imageResponse.getImage();
imageMetadata_ = imageResponse.getMetadata();
react_native_assert(status_ == ImageResponse::Status::Loading);
react_native_assert(
status_ == ImageResponse::Status::Loading ||
status_ == ImageResponse::Status::Cancelled);
status_ = ImageResponse::Status::Completed;
auto observers = observers_;
mutex_.unlock();
Expand All @@ -99,7 +103,9 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete(
void ImageResponseObserverCoordinator::nativeImageResponseFailed(
const ImageLoadError& loadError) const {
mutex_.lock();
react_native_assert(status_ == ImageResponse::Status::Loading);
react_native_assert(
status_ == ImageResponse::Status::Loading ||
status_ == ImageResponse::Status::Cancelled);
status_ = ImageResponse::Status::Failed;
imageErrorData_ = loadError.getError();
auto observers = observers_;
Expand Down

0 comments on commit e0df58d

Please sign in to comment.