Skip to content

Commit f4f1e19

Browse files
Moving more Darwin stuff to std::lock_guard lock(_lock); (project-chip#32558)
* Initial commit * Restyled by whitespace * Reverting this one * Addressing feedback * Adding a few more * Update src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm * Removing this one * Restyled by clang-format * Reverting these --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent a0ba592 commit f4f1e19

6 files changed

+48
-97
lines changed

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm

+10-20
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#import <os/lock.h>
2424

2525
#import "MTRLogging_Internal.h"
26+
#import "MTRUnfairLock.h"
27+
2628
#import <Matter/MTRAsyncCallbackWorkQueue.h>
2729

2830
#pragma mark - Class extensions
@@ -72,14 +74,10 @@ - (instancetype)initWithContext:(id)context queue:(dispatch_queue_t)queue
7274

7375
- (NSString *)description
7476
{
75-
os_unfair_lock_lock(&_lock);
77+
std::lock_guard lock(_lock);
7678

77-
auto * desc = [NSString
79+
return [NSString
7880
stringWithFormat:@"MTRAsyncCallbackWorkQueue context: %@ items count: %lu", self.context, (unsigned long) self.items.count];
79-
80-
os_unfair_lock_unlock(&_lock);
81-
82-
return desc;
8381
}
8482

8583
- (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item
@@ -91,12 +89,11 @@ - (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item
9189

9290
[item markEnqueued];
9391

94-
os_unfair_lock_lock(&_lock);
92+
std::lock_guard lock(_lock);
9593
item.workQueue = self;
9694
[self.items addObject:item];
9795

9896
[self _callNextReadyWorkItem];
99-
os_unfair_lock_unlock(&_lock);
10097
}
10198

10299
- (void)invalidate
@@ -115,11 +112,10 @@ - (void)invalidate
115112
// called after executing a work item
116113
- (void)_postProcessWorkItem:(MTRAsyncCallbackQueueWorkItem *)workItem retry:(BOOL)retry
117114
{
118-
os_unfair_lock_lock(&_lock);
115+
std::lock_guard lock(_lock);
119116
// sanity check if running
120117
if (!self.runningWorkItemCount) {
121118
// something is wrong with state - nothing is currently running
122-
os_unfair_lock_unlock(&_lock);
123119
MTR_LOG_ERROR("MTRAsyncCallbackWorkQueue endWork: no work is running on work queue");
124120
return;
125121
}
@@ -129,7 +125,6 @@ - (void)_postProcessWorkItem:(MTRAsyncCallbackQueueWorkItem *)workItem retry:(BO
129125
MTRAsyncCallbackQueueWorkItem * firstWorkItem = self.items.firstObject;
130126
if (firstWorkItem != workItem) {
131127
// something is wrong with this work item - should not be currently running
132-
os_unfair_lock_unlock(&_lock);
133128
MTR_LOG_ERROR("MTRAsyncCallbackWorkQueue endWork: work item is not first on work queue");
134129
return;
135130
}
@@ -142,7 +137,6 @@ - (void)_postProcessWorkItem:(MTRAsyncCallbackQueueWorkItem *)workItem retry:(BO
142137
// when "concurrency width" is implemented this will be decremented instead
143138
self.runningWorkItemCount = 0;
144139
[self _callNextReadyWorkItem];
145-
os_unfair_lock_unlock(&_lock);
146140
}
147141

148142
- (void)endWork:(MTRAsyncCallbackQueueWorkItem *)workItem
@@ -203,34 +197,30 @@ - (void)_invalidate
203197

204198
- (void)invalidate
205199
{
206-
os_unfair_lock_lock(&_lock);
200+
std::lock_guard lock(_lock);
207201
[self _invalidate];
208-
os_unfair_lock_unlock(&_lock);
209202
}
210203

211204
- (void)markEnqueued
212205
{
213-
os_unfair_lock_lock(&_lock);
206+
std::lock_guard lock(_lock);
214207
_enqueued = YES;
215-
os_unfair_lock_unlock(&_lock);
216208
}
217209

218210
- (void)setReadyHandler:(MTRAsyncCallbackReadyHandler)readyHandler
219211
{
220-
os_unfair_lock_lock(&_lock);
212+
std::lock_guard lock(_lock);
221213
if (!_enqueued) {
222214
_readyHandler = readyHandler;
223215
}
224-
os_unfair_lock_unlock(&_lock);
225216
}
226217

227218
- (void)setCancelHandler:(dispatch_block_t)cancelHandler
228219
{
229-
os_unfair_lock_lock(&_lock);
220+
std::lock_guard lock(_lock);
230221
if (!_enqueued) {
231222
_cancelHandler = cancelHandler;
232223
}
233-
os_unfair_lock_unlock(&_lock);
234224
}
235225

236226
- (void)endWork

src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm

+6-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#import "MTRAsyncWorkQueue.h"
1919
#import "MTRDefines_Internal.h"
2020
#import "MTRLogging_Internal.h"
21+
#import "MTRUnfairLock.h"
2122

2223
#import <atomic>
2324
#import <os/lock.h>
@@ -273,13 +274,12 @@ - (void)enqueueWorkItem:(MTRAsyncWorkItem *)item
273274
- (void)invalidate
274275
{
275276
ContextSnapshot context(_context); // outside of lock
276-
os_unfair_lock_lock(&_lock);
277+
std::lock_guard lock(_lock);
277278
MTR_LOG_INFO("MTRAsyncWorkQueue<%@> invalidate %tu items", context.description, _items.count);
278279
for (MTRAsyncWorkItem * item in _items) {
279280
[item cancel];
280281
}
281282
[_items removeAllObjects];
282-
os_unfair_lock_unlock(&_lock);
283283
}
284284

285285
- (void)_postProcessWorkItem:(MTRAsyncWorkItem *)workItem
@@ -376,8 +376,7 @@ - (void)_callNextReadyWorkItemWithContext:(ContextSnapshot const &)context
376376

377377
- (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData
378378
{
379-
BOOL hasDuplicate = NO;
380-
os_unfair_lock_lock(&_lock);
379+
std::lock_guard lock(_lock);
381380
// Start from the last item
382381
for (MTRAsyncWorkItem * item in [_items reverseObjectEnumerator]) {
383382
auto duplicateCheckHandler = item.duplicateCheckHandler;
@@ -386,13 +385,12 @@ - (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id
386385
BOOL isDuplicate = NO;
387386
duplicateCheckHandler(opaqueWorkItemData, &isDuplicate, &stop);
388387
if (stop) {
389-
hasDuplicate = isDuplicate;
390-
break;
388+
return isDuplicate;
391389
}
392390
}
393391
}
394-
os_unfair_lock_unlock(&_lock);
395-
return hasDuplicate;
392+
393+
return NO;
396394
}
397395

398396
@end

0 commit comments

Comments
 (0)