Darwin: Refactor MTRAsyncWorkQueue to avoid retain cycles (#29464)

* Darwin: Refactor MTRAsyncWorkQueue to avoid retain cycles

- Pass a completion to readyHandler instead of capturing the work item
- Hold the context weakly within the queue and handle context loss
- Simplify thread-safety by having clear ownership rules for work items

* Darwin: Consolidate MTRAsyncWorkQueue headers

The internal header is no longer needed because MTRAsyncWorkQueue is entirely
private to the project now. Also tidy up duplicate / batching logic a little.

* Darwin: invalidate MTRDevice work queue

* Address review comments

* Add os_unfair_lock_assert_owner and comments

* Introduce MTRAsyncWorkItemRetryCountBase

* More tweaks from review

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm

@@ -47,7 +47,7 @@ @interface MTRAsyncCallbackQueueWorkItem ()
 @property (nonatomic, strong) MTRAsyncCallbackWorkQueue * workQueue;
 @property (nonatomic, readonly) BOOL enqueued;
 // Called by the queue
-- (void)markedEnqueued;
+- (void)markEnqueued;
 - (void)callReadyHandlerWithContext:(id)context;
 - (void)cancel;
 @end
@@ -85,7 +85,7 @@ - (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item
         return;
     }
 
-    [item markedEnqueued];
+    [item markEnqueued];
 
     os_unfair_lock_lock(&_lock);
     item.workQueue = self;
@@ -204,7 +204,7 @@ - (void)invalidate
     os_unfair_lock_unlock(&_lock);
 }
 
-- (void)markedEnqueued
+- (void)markEnqueued
 {
     os_unfair_lock_lock(&_lock);
     _enqueued = YES;

src/darwin/Framework/CHIP/MTRAsyncWorkQueue.h

@@ -19,66 +19,187 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@class MTRAsyncWorkItem;
-
-typedef void (^MTRAsyncWorkReadyHandler)(id context, NSUInteger retryCount);
-
-// MTRAsyncWorkQueue high level description
-//   The MTRAsyncWorkQueue was made to call one readyHandler
-//   block at a time asynchronously, and the readyHandler is
-//   expected to start/schedule a task. When the task finishes
-//   asynchronously in the future (at any time, from any queue
-//   or thread), it is expected to ask the workItem object to
-//   either endWork or retryWork.
-
-// Sequence of steps when queuing a work item:
-//   - Create MTRAsyncWorkItem object
-//   - Create ready handler block (MTRAsyncWorkReadyHandler)
-//      - block is called when it's the WorkItem's turn to do work
-//      - its body is to perform a task that is expected to end asynchronously in the future
-//      - at the end of work, call on the work item object:
-//         - endWork for success or failure
-//         - retryWork for temporary failures
-//   - Set the readyHandler block on the WorkItem object
-//   - Call enqueueWorkItem on a MTRAsyncWorkQueue
-
-// A serial one-at-a-time queue for performing work items
+typedef NS_ENUM(NSInteger, MTRAsyncWorkOutcome) {
+    MTRAsyncWorkComplete,
+    MTRAsyncWorkNeedsRetry,
+};
+
+/// The type of completion handler passed to `MTRAsyncWorkItem`.
+/// Return YES if the completion call was valid, or NO if the
+/// work item was already completed previously (e.g. due to
+/// being cancelled).
+typedef BOOL (^MTRAsyncWorkCompletionBlock)(MTRAsyncWorkOutcome outcome);
+
+/// An optional handler that controls batching of MTRAsyncWorkItem.
+///
+/// When a work item is dequeued to run, if it is of a type that can be
+/// combined with similar work items in a batch, this facility provides an
+/// opportunity to coalesce and merge work items.
+///
+/// The batching handler is called by the work queue when all of the following
+/// are true:
+/// 1) A work item that is batchable is about to be executed for the first time
+/// 2) The next work item in the queue is also batchable
+/// 3) The two work items have identical batching ids
+///
+/// The handler will be passed the opaque data of the two work items:
+/// `opaqueDataCurrent` is the data of the item about to be executed and
+/// `opaqueDataNext` is the data for the next item. The `fullyMerged` parameter
+/// will be initialized to NO by the caller.
+///
+/// The handler is expected to mutate the data as needed to achieve batching.
+///
+/// If after the data mutations opaqueDataNext no longer requires any work, the
+/// handler should set `fullyMerged` to YES to indicate that the next item can
+/// be dropped from the queue. In this case, the handler may be called again to
+/// possibly also batch the work item after the one that was dropped.
+///
+/// @see MTRAsyncWorkItem
+typedef void (^MTRAsyncWorkBatchingHandler)(id opaqueDataCurrent, id opaqueDataNext, BOOL * fullyMerged);
+
+/// An optional handler than enables duplicate checking for MTRAsyncWorkItem.
+///
+/// The duplicate check handler is called when the client wishes to check
+/// whether a new candidate work item is a duplicate of an existing queued
+/// item, so that the client may decide to not enqueue the duplicate work.
+/// Duplicate checking is performed in reverse queue order, i.e. more
+/// recently enqueued items will be checked first.
+///
+/// The handler will be passed the opaque data of the candidate work item. The
+/// `stop` and `isDuplicate` parameters will be initialized to NO by the caller.
+///
+/// If the handler determines the data is indeed duplicate work, it should
+/// set `stop` to YES, and set `isDuplicate` to YES.
+///
+/// If the handler determines the data is not duplicate work, it should set
+/// `stop` to YES, and set `isDuplicate` to NO.
+///
+/// If the handler is unable to determine if the data is duplicate work, it
+/// should set `stop` to NO; the value of `isDuplicate` will be ignored.
+///
+/// @see MTRAsyncWorkItem
+typedef void (^MTRAsyncWorkDuplicateCheckHandler)(id opaqueItemData, BOOL * isDuplicate, BOOL * stop);
+
+/// A unit of work that can be run on a `MTRAsyncWorkQueue`.
+///
+/// A work item can be configured with a number of hander blocks called by the
+/// async work queue in various situations. Generally work items will have at
+/// least a `readyHandler` (though it is technically optional).
+///
+/// This class is not thread-safe, and once a work item has been submitted to
+/// the queue via `enqueueWorkItem` ownership of the work item passes to the
+/// queue. No further modifications may be made to it after that point.
+///
+/// @see -[MTRAsyncWorkQueue enqueueWorkItem:]
 MTR_TESTABLE
-@interface MTRAsyncWorkQueue : NSObject
+@interface MTRAsyncWorkItem<__contravariant ContextType> : NSObject
+
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
 
-// The context object is only held and passed back as a reference and is opaque to the work queue
-- (instancetype)initWithContext:(id _Nullable)context queue:(dispatch_queue_t)queue;
+/// Creates a work item that will run on the specified dispatch queue.
+- (instancetype)initWithQueue:(dispatch_queue_t)queue;
 
-// Called by the work queue owner to clean up and cancel work items
-- (void)invalidate;
+/// Called by the work queue to start this work item.
+///
+/// Will be called on the dispatch queue associated with this item.
+///
+/// This handler block must, synchronously or asynchronously from any thread,
+/// call the provided completion block exactly once. Passing an outcome of
+/// MTRAsyncWorkComplete removes it from the queue and allows the queue to move
+/// on to the next work item (if any).
+///
+/// Passing an outcome of MTRAsyncWorkNeedsRetry causes the queue to start the
+/// work item again with an incremented retryCount. The retryCount is 0 when a
+/// work item is executed for the first time.
+@property (nonatomic, strong, nullable) void (^readyHandler)
+    (ContextType context, NSInteger retryCount, MTRAsyncWorkCompletionBlock completion);
+
+/// Called by the work queue to cancel the work item.
+///
+/// Will be called on the dispatch queue associated with this item.
+/// The work item may or may not have been started already.
+@property (nonatomic, strong, nullable) void (^cancelHandler)(void);
+
+@property (nonatomic, readonly) NSUInteger batchingID;
+@property (nonatomic, readonly, nullable) id batchableData;
+@property (nonatomic, readonly, nullable) MTRAsyncWorkBatchingHandler batchingHandler;
+
+/// Sets the batching handler and associated data for this work item.
+///
+/// Note: This handler is NOT called on the dispatch queue associated with
+/// this work item. Thread-safety is managed by the work queue internally.
+///
+/// If no `batchingHandler` is set using this method, the work item will not
+/// participate in batching, and the `batchingID` and `batchableData`
+/// properties are  meaningless.
+///
+/// @see MTRAsyncWorkBatchingHandler
+- (void)setBatchingID:(NSUInteger)opaqueBatchingID
+                 data:(id)opaqueBatchableData
+              handler:(MTRAsyncWorkBatchingHandler)batchingHandler;
+
+@property (nonatomic, readonly) NSUInteger duplicateTypeID;
+@property (nonatomic, readonly, nullable) MTRAsyncWorkDuplicateCheckHandler duplicateCheckHandler;
+
+/// Sets the duplicate check type and handler for this work item.
+///
+/// Note: This handler is NOT called on the dispatch queue associated with
+/// this work item. Thread-safety is managed by the work queue internally.
+///
+/// If no `duplicateCheckHandler` is set using this method, the work item
+/// will not participate in duplicate checking, and the `duplicateTypeID`
+/// property is meaningless.
+///
+/// @see MTRAsyncWorkDuplicateCheckHandler
+- (void)setDuplicateTypeID:(NSUInteger)opaqueDuplicateTypeID
+                   handler:(MTRAsyncWorkDuplicateCheckHandler)duplicateCheckHandler;
 
-// Work items may be enqueued from any queue or thread
-// Note: Once a work item is enqueued, its handlers cannot be modified
-- (void)enqueueWorkItem:(MTRAsyncWorkItem *)item;
 @end
 
-// An item in the work queue
+/// A serial one-at-a-time queue for performing asynchronous work items.
+///
+/// Units of work are represented by MTRAsyncWorkItem objects that are
+/// configured with one or more handler blocks before being passed to
+/// `enqueueWorkItem:`.
+///
+/// MTRAsyncWorkQueue is thread-safe.
 MTR_TESTABLE
-@interface MTRAsyncWorkItem : NSObject
+@interface MTRAsyncWorkQueue<ContextType> : NSObject
+
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
 
-// Both readyHandler and cancelHander will be called on the queue given to initWithQueue
-- (instancetype)initWithQueue:(dispatch_queue_t)queue;
-@property (nonatomic, strong) MTRAsyncWorkReadyHandler readyHandler;
-@property (nonatomic, strong) dispatch_block_t cancelHandler;
-
-// Called by the creater of the work item when async work is done and should
-// be removed from the queue. The work queue will run the next work item.
-// Note: This must only be called from within the readyHandler
-- (void)endWork;
-
-// Called by the creater of the work item when async work should be retried.
-// The work queue will call this workItem's readyHandler again.
-// Note: This must only be called from within the readyHandler
-- (void)retryWork;
+/// Creates a work queue with the given context object.
+///
+/// The context object is weakly held and passed to the readyHandler of work
+/// items. This avoids work item blocks accidentally creating a retain cycle
+/// by strongly closing over the context object themselves (since the context
+/// object will generally be holding a strong reference to the work queue
+/// itself). The owner of the queue is responsible for keeping the context
+/// object alive; no further work items will be executed if the context object
+/// is lost.
+- (instancetype)initWithContext:(ContextType)context;
+
+/// Enqueues the specified work item, making it eligible for execution.
+///
+/// Once a work item is enqueued, ownership of it passes to the queue and
+/// no further modifications may be made to it. Work item objects cannot be
+/// re-used.
+- (void)enqueueWorkItem:(MTRAsyncWorkItem<ContextType> *)item;
+
+/// Checks whether the queue already contains a work item matching the provided
+/// details. A client may call this method to avoid enqueueing duplicate work.
+///
+/// This method will call the duplicate check handler for all work items
+/// matching the duplicate type ID, starting from the last item in the queue
+///
+/// @see MTRAsyncWorkDuplicateCheckHandler
+- (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID
+                 workItemData:(id)opaqueWorkItemData;
+
+/// Cancels and removes all work items.
+- (void)invalidate;
 @end
 
 NS_ASSUME_NONNULL_END