From 0ec2d526f35374dc0366024f8bb62aac4db1fd8d Mon Sep 17 00:00:00 2001 From: Mark Davis Date: Fri, 25 Oct 2024 12:55:13 -0700 Subject: [PATCH] Disable automatic async conversion for IGListAdapter APIs Summary: Threads has now run into multiple instances of device-only crashes resulting from usage of automatically converted Swift async versions of `performUpdates(animated:)` and `reloadData()`. See D64428846, D64876801. The crashes appear to stem from the opaque, automatically generated async methodology: ``` objc completion handler block implementation for escaping callee_unowned convention(block) (unowned Bool) -> () with result type Bool ``` Our `IGListKit` infra assuming ownership of these `callee_unowned` blocks seems to be problematic: the block can be released/nil'd, and our downstream [`[-[IGListReloadTransaction _executeCompletionBlocks:]`](https://www.internalfb.com/intern/logview/redirect/?filename=IGListReloadTransaction.m&line=82&repo=fbsource&revision=c1e4b4a34ff90e4785983b260f10543af6536215&build=655525612) execution then crashes. Given this incompatibility with automatic conversion, here I propose: 1. marking the relevant `IGListAdapter.h` APIs with `NS_SWIFT_DISABLE_ASYNC` 2. providing manual async APIs in a new `IGListAdapter+Async.swift` extension hosted in `IGListSwiftKit` that existing callers can now make use of. Differential Revision: D64881920 fbshipit-source-id: b15a5141a79491c1871b92f4f84a0c29b6045ee0 --- Source/IGListKit/IGListAdapter.h | 4 +- .../IGListSwiftKit/IGListAdapter+Async.swift | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 Source/IGListSwiftKit/IGListAdapter+Async.swift diff --git a/Source/IGListKit/IGListAdapter.h b/Source/IGListKit/IGListAdapter.h index 1e00d190e..dcf50f469 100644 --- a/Source/IGListKit/IGListAdapter.h +++ b/Source/IGListKit/IGListAdapter.h @@ -143,7 +143,7 @@ NS_SWIFT_NAME(ListAdapter) @param animated A flag indicating if the transition should be animated. @param completion The block to execute when the updates complete. */ -- (void)performUpdatesAnimated:(BOOL)animated completion:(nullable IGListUpdaterCompletion)completion; +- (void)performUpdatesAnimated:(BOOL)animated completion:(nullable IGListUpdaterCompletion)completion NS_SWIFT_DISABLE_ASYNC; /** Perform an immediate reload of the data in the data source, discarding the old objects. @@ -153,7 +153,7 @@ NS_SWIFT_NAME(ListAdapter) @warning Do not use this method to update without animations as it can be very expensive to teardown and rebuild all section controllers. Use `-[IGListAdapter performUpdatesAnimated:completion]` instead. */ -- (void)reloadDataWithCompletion:(nullable IGListUpdaterCompletion)completion; +- (void)reloadDataWithCompletion:(nullable IGListUpdaterCompletion)completion NS_SWIFT_DISABLE_ASYNC; /** Reload the list for only the specified objects. diff --git a/Source/IGListSwiftKit/IGListAdapter+Async.swift b/Source/IGListSwiftKit/IGListAdapter+Async.swift new file mode 100644 index 000000000..c1a4b2888 --- /dev/null +++ b/Source/IGListSwiftKit/IGListAdapter+Async.swift @@ -0,0 +1,39 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import IGListKit + +public extension ListAdapter { + /// Perform an update from the previous state of the data source. This is analogous to calling + /// `UICollectionView.performBatchUpdates(_:completion:)`. + /// + /// - Parameter animated: A flag indicating if the transition should be animated. + /// - Returns: `true` if the update animations completed successfully; otherwise `false`. + @discardableResult + func performUpdates(animated: Bool) async -> Bool { + return await withCheckedContinuation { continuation in + performUpdates(animated: animated) { finished in + continuation.resume(returning: finished) + } + } + } + + /// Perform an immediate reload of the data in the data source, discarding the old objects. + /// + /// - Returns: `true` if the update animations completed successfully; otherwise `false`. + /// + /// @warning Do not use this method to update without animations as it can be very expensive to teardown and rebuild all + /// section controllers. Use `performUpdates(animated:) async` instead. + @discardableResult + func reloadData() async -> Bool { + return await withCheckedContinuation { continuation in + reloadData { finished in + continuation.resume(returning: finished) + } + } + } +}