Add nullptr check to SharedFunction (#38075)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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
This commit is contained in:
Родитель
a702d0515f
Коммит
ebbd22c9d8
|
@ -109,6 +109,9 @@ using namespace facebook::react;
|
|||
// 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();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -46,7 +46,7 @@ class ImageShadowNode final : public ConcreteViewShadowNode<
|
|||
ShadowNodeFamily::Shared const & /*family*/,
|
||||
ComponentDescriptor const &componentDescriptor) {
|
||||
auto imageSource = ImageSource{ImageSource::Type::Invalid};
|
||||
return {imageSource, {imageSource, nullptr}, 0};
|
||||
return {imageSource, {imageSource, nullptr, {}}, 0};
|
||||
}
|
||||
|
||||
#pragma mark - LayoutableShadowNode
|
||||
|
|
|
@ -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::react {
|
||||
|
||||
|
@ -29,7 +30,8 @@ class ImageRequest final {
|
|||
*/
|
||||
ImageRequest(
|
||||
ImageSource imageSource,
|
||||
std::shared_ptr<const ImageTelemetry> telemetry);
|
||||
std::shared_ptr<const ImageTelemetry> telemetry,
|
||||
SharedFunction<> cancelationFunction);
|
||||
|
||||
/*
|
||||
* The move constructor.
|
||||
|
@ -41,11 +43,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.
|
||||
|
@ -94,9 +91,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 facebook::react
|
||||
|
|
|
@ -24,7 +24,7 @@ ImageRequest ImageManager::requestImage(
|
|||
const ImageSource &imageSource,
|
||||
SurfaceId /*surfaceId*/) const {
|
||||
// Not implemented.
|
||||
return {imageSource, nullptr};
|
||||
return {imageSource, nullptr, {}};
|
||||
}
|
||||
|
||||
} // namespace facebook::react
|
||||
|
|
|
@ -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.
|
||||
}
|
||||
|
||||
|
|
|
@ -11,20 +11,16 @@ 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)) {
|
||||
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 {
|
||||
|
|
|
@ -48,13 +48,11 @@ using namespace facebook::react;
|
|||
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
|
||||
|
|
|
@ -37,13 +37,11 @@ using namespace facebook::react;
|
|||
- (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);
|
||||
|
|
|
@ -21,9 +21,9 @@ namespace facebook::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)) {}
|
||||
|
@ -46,9 +46,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:
|
||||
|
|
Загрузка…
Ссылка в новой задаче