Add nullptr check to SharedFunction (#38075)

Summary:
Pull Request resolved: #38075

changelog: [internal]

`SharedFunction<>` is created with nullptr for its internal `std::function`. If called after created with default constructor, it crashes app. It also does not have API to check if its internal function is not nullptr.

With image cancelation, there is a race between when native component calls `imageRequest.cancel()` and when cancelation function is set in `RCTImageManager`.
To fix this, this diff adds a nullptr check inside SharedFunction. So it is always safe to call.

Reviewed By: javache

Differential Revision: D47022957

fbshipit-source-id: 0a04a87cd1ffe6bf3ca2fded38f689f06cc92ca9

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

@@ -109,6 +109,9 @@ - (void)_setStateAndResubscribeImageResponseObserver:(ImageShadowNode::ConcreteS
       // This will only become issue if we decouple life cycle of a
       // ShadowNode from ComponentView, which is not something we do now.
       imageRequest.cancel();
+      imageRequest.cancel();
+      imageRequest.cancel();
+      imageRequest.cancel();
     }
   }
 

packages/react-native/ReactCommon/react/renderer/components/image/ImageShadowNode.h

@@ -46,7 +46,7 @@ class ImageShadowNode final : public ConcreteViewShadowNode<
       ShadowNodeFamilyFragment const &familyFragment,
       ComponentDescriptor const &componentDescriptor) {
     auto imageSource = ImageSource{ImageSource::Type::Invalid};
-    return {imageSource, {imageSource, nullptr}, 0};
+    return {imageSource, {imageSource, nullptr, {}}, 0};
   }
 
 #pragma mark - LayoutableShadowNode

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

@@ -12,6 +12,7 @@
 #include <react/renderer/imagemanager/ImageResponseObserverCoordinator.h>
 #include <react/renderer/imagemanager/ImageTelemetry.h>
 #include <react/renderer/imagemanager/primitives.h>
+#include <react/utils/SharedFunction.h>
 
 namespace facebook {
 namespace react {
@@ -30,7 +31,8 @@ class ImageRequest final {
    */
   ImageRequest(
       ImageSource imageSource,
-      std::shared_ptr<const ImageTelemetry> telemetry);
+      std::shared_ptr<const ImageTelemetry> telemetry,
+      SharedFunction<> cancelationFunction);
 
   /*
    * The move constructor.
@@ -42,11 +44,6 @@ class ImageRequest final {
    */
   ImageRequest(const ImageRequest &other) = delete;
 
-  /**
-   * Set cancelation function.
-   */
-  void setCancelationFunction(std::function<void(void)> cancelationFunction);
-
   /*
    * Calls cancel function if one is defined. Should be when downloading
    * image isn't needed anymore. E.g. <ImageView /> was removed.
@@ -95,9 +92,9 @@ class ImageRequest final {
   std::shared_ptr<const ImageResponseObserverCoordinator> coordinator_{};
 
   /*
-   * Function we can call to cancel image request (see destructor).
+   * Function we can call to cancel image request.
    */
-  std::function<void(void)> cancelRequest_;
+  SharedFunction<> cancelRequest_;
 };
 
 } // namespace react

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/cxx/react/renderer/imagemanager/ImageManager.cpp

@@ -24,7 +24,7 @@ ImageRequest ImageManager::requestImage(
     const ImageSource &imageSource,
     SurfaceId /*surfaceId*/) const {
   // Not implemented.
-  return {imageSource, nullptr};
+  return {imageSource, nullptr, {}};
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/cxx/react/renderer/imagemanager/ImageRequest.cpp

@@ -13,8 +13,11 @@ namespace facebook::react {
 
 ImageRequest::ImageRequest(
     ImageSource imageSource,
-    std::shared_ptr<const ImageTelemetry> telemetry)
-    : imageSource_(std::move(imageSource)), telemetry_(std::move(telemetry)) {
+    std::shared_ptr<const ImageTelemetry> telemetry,
+    SharedFunction<> cancelationFunction)
+    : imageSource_(std::move(imageSource)),
+      telemetry_(std::move(telemetry)),
+      cancelRequest_(std::move(cancelationFunction)) {
   // Not implemented.
 }
 

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/ios/react/renderer/imagemanager/ImageRequest.cpp

@@ -12,20 +12,16 @@ namespace react {
 
 ImageRequest::ImageRequest(
     ImageSource imageSource,
-    std::shared_ptr<const ImageTelemetry> telemetry)
-    : imageSource_(std::move(imageSource)), telemetry_(std::move(telemetry)) {
+    std::shared_ptr<const ImageTelemetry> telemetry,
+    SharedFunction<> cancelationFunction)
+    : imageSource_(std::move(imageSource)),
+      telemetry_(std::move(telemetry)),
+      cancelRequest_(std::move(cancelationFunction)) {
   coordinator_ = std::make_shared<ImageResponseObserverCoordinator>();
 }
 
-void ImageRequest::setCancelationFunction(
-    std::function<void(void)> cancelationFunction) {
-  cancelRequest_ = cancelationFunction;
-}
-
 void ImageRequest::cancel() const {
-  if (cancelRequest_) {
-    cancelRequest_();
-  }
+  cancelRequest_();
 }
 
 const ImageSource &ImageRequest::getImageSource() const {

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/ios/react/renderer/imagemanager/RCTImageManager.mm

@@ -48,13 +48,11 @@ - (ImageRequest)requestImage:(ImageSource)imageSource surfaceId:(SurfaceId)surfa
     telemetry = nullptr;
   }
 
-  auto imageRequest = ImageRequest(imageSource, telemetry);
+  auto sharedCancelationFunction = SharedFunction<>();
+  auto imageRequest = ImageRequest(imageSource, telemetry, sharedCancelationFunction);
   auto weakObserverCoordinator =
       (std::weak_ptr<const ImageResponseObserverCoordinator>)imageRequest.getSharedObserverCoordinator();
 
-  auto sharedCancelationFunction = SharedFunction<>();
-  imageRequest.setCancelationFunction(sharedCancelationFunction);
-
   /*
    * Even if an image is being loaded asynchronously on some other background thread, some other preparation
    * work (such as creating an `NSURLRequest` object and some obscure logic inside `RCTImageLoader`) can take a couple

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/ios/react/renderer/imagemanager/RCTSyncImageManager.mm

@@ -37,13 +37,11 @@ - (instancetype)initWithImageLoader:(id<RCTImageLoaderWithAttributionProtocol>)i
 - (ImageRequest)requestImage:(ImageSource)imageSource surfaceId:(SurfaceId)surfaceId
 {
   auto telemetry = std::make_shared<ImageTelemetry>(surfaceId);
-  auto imageRequest = ImageRequest(imageSource, telemetry);
+  auto sharedCancelationFunction = SharedFunction<>();
+  auto imageRequest = ImageRequest(imageSource, telemetry, sharedCancelationFunction);
   auto weakObserverCoordinator =
       (std::weak_ptr<const ImageResponseObserverCoordinator>)imageRequest.getSharedObserverCoordinator();
 
-  auto sharedCancelationFunction = SharedFunction<>();
-  imageRequest.setCancelationFunction(sharedCancelationFunction);
-
   dispatch_group_t imageWaitGroup = dispatch_group_create();
 
   dispatch_group_enter(imageWaitGroup);

packages/react-native/ReactCommon/react/utils/SharedFunction.h

@@ -22,9 +22,9 @@ namespace react {
  * - When captured by `std::function` arguments are not copyable;
  * - When we need to replace the content of the callable later on the go.
  */
-template <typename ReturnT = void, typename... ArgumentT>
+template <typename... ArgumentT>
 class SharedFunction {
-  using T = ReturnT(ArgumentT...);
+  using T = void(ArgumentT...);
 
   struct Pair {
     Pair(std::function<T> &&function) : function(std::move(function)) {}
@@ -47,9 +47,11 @@ class SharedFunction {
     pair_->function = function;
   }
 
-  ReturnT operator()(ArgumentT... args) const {
+  void operator()(ArgumentT... args) const {
     std::shared_lock lock(pair_->mutex);
-    return pair_->function(args...);
+    if (pair_->function) {
+      pair_->function(args...);
+    }
   }
 
  private: