Skip to content

Commit

Permalink
Disable automatic async conversion for IGListAdapter APIs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Mark Davis authored and facebook-github-bot committed Oct 25, 2024
1 parent 636f6b5 commit 0ec2d52
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
4 changes: 2 additions & 2 deletions Source/IGListKit/IGListAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
39 changes: 39 additions & 0 deletions Source/IGListSwiftKit/IGListAdapter+Async.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}

0 comments on commit 0ec2d52

Please # to comment.