Address review comments

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

@@ -33,8 +33,8 @@ 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 gives the
-/// provides an opportunity to coalesce and merge work items.
+/// 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:
@@ -84,9 +84,9 @@ typedef void (^MTRAsyncWorkDuplicateCheckHandler)(id opaqueItemData, BOOL * isDu
 /// 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 be 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.
+/// 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
@@ -120,8 +120,8 @@ MTR_TESTABLE
 @property (nonatomic, strong, nullable) void (^cancelHandler)(void);
 
 @property (nonatomic, readonly) NSUInteger batchingID;
-@property (nonatomic, readonly) id batchableData;
-@property (nonatomic, readonly) MTRAsyncWorkBatchingHandler batchingHandler;
+@property (nonatomic, readonly, nullable) id batchableData;
+@property (nonatomic, readonly, nullable) MTRAsyncWorkBatchingHandler batchingHandler;
 
 /// Sets the batching handler and associated data for this work item.
 ///
@@ -134,7 +134,7 @@ MTR_TESTABLE
               handler:(MTRAsyncWorkBatchingHandler)batchingHandler;
 
 @property (nonatomic, readonly) NSUInteger duplicateTypeID;
-@property (nonatomic, readonly) MTRAsyncWorkDuplicateCheckHandler duplicateCheckHandler;
+@property (nonatomic, readonly, nullable) MTRAsyncWorkDuplicateCheckHandler duplicateCheckHandler;
 
 /// Sets the duplicate check type and handler for this work item.
 ///

src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm

@@ -62,7 +62,7 @@ - (void)setCancelHandler:(void (^)(void))cancelHandler
     _cancelHandler = cancelHandler;
 }
 
-- (void)markedEnqueued
+- (void)markEnqueued
 {
     [self assertMutable];
     _state = MTRAsyncWorkItemEnqueued;
@@ -114,13 +114,19 @@ - (void)markComplete
 {
     NSAssert(_state >= MTRAsyncWorkItemEnqueued, @"work item was not enqueued (%ld)", (long) _state);
     _state = MTRAsyncWorkItemComplete;
+
+    // Clear all handlers in case any of them captured this object.
+    _readyHandler = nil;
+    _cancelHandler = nil;
+    _batchingHandler = nil;
+    _duplicateCheckHandler = nil;
 }
 
 - (void)cancel
 {
     if (_state != MTRAsyncWorkItemComplete) {
-        _state = MTRAsyncWorkItemComplete;
         auto cancelHandler = _cancelHandler;
+        [self markComplete];
         if (cancelHandler) {
             // Note that this does not prevent a race against
             // the readyHandler calling the work completion.
@@ -178,7 +184,7 @@ - (void)enqueueWorkItem:(MTRAsyncWorkItem *)item
 {
     NSParameterAssert(item);
     NSAssert(_context, @"context has been lost");
-    [item markedEnqueued];
+    [item markEnqueued];
 
     os_unfair_lock_lock(&_lock);
     [_items addObject:item];

src/darwin/Framework/CHIPTests/MTRAsyncWorkQueueTests.m

@@ -58,7 +58,7 @@ - (void)testRunItem
         XCTAssertNil(weakItem);
     }];
 
-    [self waitForExpectationsWithTimeout:0.5 handler:nil];
+    [self waitForExpectationsWithTimeout:1 handler:nil];
 
     // see that it only ran once
     XCTAssertEqual(counter, 1);