From 0e10ee6ac62991bcb6e792b422ced77e752fae77 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Tue, 3 Oct 2023 07:55:17 -0700 Subject: [PATCH] Allow AsyncCallback to be used with lambda (#39717) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39717 AsyncCallback is a better abstraction than the current usage of WeakCallbackWrapper and RAIICallbackWrapperDestroyer, and has way fewer gotchas. Making a few changes here to make it easier to use in various scenarios and match the behaviour we're already seeing in CallbackWrapper. 1) Remove the explicit copy constructor, since this prevents an automatic move constructor from being generated 2) Add a call variant which takes a lambda, for callers which need to manually create JSI arguments 3) Ignore AsyncCallback invocations when the underlying runtime has gone away. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D49684248 fbshipit-source-id: 8b49ec22cc409572ead80a85b10a190994bf0dd5 --- .../ReactCommon/react/bridging/Function.h | 70 ++++++++++++++----- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/packages/react-native/ReactCommon/react/bridging/Function.h b/packages/react-native/ReactCommon/react/bridging/Function.h index f479c509dc4e7a..d934ddbca35a5e 100644 --- a/packages/react-native/ReactCommon/react/bridging/Function.h +++ b/packages/react-native/ReactCommon/react/bridging/Function.h @@ -29,19 +29,27 @@ class AsyncCallback { std::move(function), std::move(jsInvoker))) {} - AsyncCallback(const AsyncCallback&) = default; - AsyncCallback& operator=(const AsyncCallback&) = default; - void operator()(Args... args) const { call(std::forward(args)...); } void call(Args... args) const { - callInternal(std::nullopt, std::forward(args)...); + callWithArgs(std::nullopt, std::forward(args)...); } void callWithPriority(SchedulerPriority priority, Args... args) const { - callInternal(priority, std::forward(args)...); + callWithArgs(priority, std::forward(args)...); + } + + void call( + std::function&& callImpl) const { + callWithFunction(std::nullopt, std::move(callImpl)); + } + + void callWithPriority( + SchedulerPriority priority, + std::function&& callImpl) const { + callWithFunction(priority, std::move(callImpl)); } private: @@ -49,22 +57,39 @@ class AsyncCallback { std::shared_ptr> callback_; - void callInternal(std::optional priority, Args... args) + void callWithArgs(std::optional priority, Args... args) const { auto wrapper = callback_->wrapper_.lock(); - if (!wrapper) { - throw std::runtime_error("Failed to call invalidated async callback"); + if (wrapper) { + auto fn = [callback = callback_, + argsPtr = std::make_shared>( + std::make_tuple(std::forward(args)...))] { + callback->apply(std::move(*argsPtr)); + }; + + if (priority) { + wrapper->jsInvoker().invokeAsync(*priority, std::move(fn)); + } else { + wrapper->jsInvoker().invokeAsync(std::move(fn)); + } } - auto fn = [callback = callback_, - argsPtr = std::make_shared>( - std::make_tuple(std::forward(args)...))] { - callback->apply(std::move(*argsPtr)); - }; - - if (priority) { - wrapper->jsInvoker().invokeAsync(*priority, std::move(fn)); - } else { - wrapper->jsInvoker().invokeAsync(std::move(fn)); + } + + void callWithFunction( + std::optional priority, + std::function&& callImpl) const { + auto wrapper = callback_->wrapper_.lock(); + if (wrapper) { + auto fn = [wrapper = std::move(wrapper), + callImpl = std::move(callImpl)]() { + callImpl(wrapper->runtime(), wrapper->callback()); + }; + + if (priority) { + wrapper->jsInvoker().invokeAsync(*priority, std::move(fn)); + } else { + wrapper->jsInvoker().invokeAsync(std::move(fn)); + } } } }; @@ -97,8 +122,15 @@ class SyncCallback { R call(Args... args) const { auto wrapper = wrapper_.lock(); + + // If the wrapper has been deallocated, we can no longer provide a return + // value consistently, so our only option is to throw if (!wrapper) { - throw std::runtime_error("Failed to call invalidated sync callback"); + if constexpr (std::is_void_v) { + return; + } else { + throw std::runtime_error("Failed to call invalidated sync callback"); + } } auto& callback = wrapper->callback();