Moving more Darwin stuff to std::lock_guard lock(_lock); (#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>
diff --git a/src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm b/src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm
index 00b6d33..1e44b4a 100644
--- a/src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm
+++ b/src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm
@@ -23,6 +23,8 @@
#import <os/lock.h>
#import "MTRLogging_Internal.h"
+#import "MTRUnfairLock.h"
+
#import <Matter/MTRAsyncCallbackWorkQueue.h>
#pragma mark - Class extensions
@@ -72,14 +74,10 @@
- (NSString *)description
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
- auto * desc = [NSString
+ return [NSString
stringWithFormat:@"MTRAsyncCallbackWorkQueue context: %@ items count: %lu", self.context, (unsigned long) self.items.count];
-
- os_unfair_lock_unlock(&_lock);
-
- return desc;
}
- (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item
@@ -91,12 +89,11 @@
[item markEnqueued];
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
item.workQueue = self;
[self.items addObject:item];
[self _callNextReadyWorkItem];
- os_unfair_lock_unlock(&_lock);
}
- (void)invalidate
@@ -115,11 +112,10 @@
// called after executing a work item
- (void)_postProcessWorkItem:(MTRAsyncCallbackQueueWorkItem *)workItem retry:(BOOL)retry
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
// sanity check if running
if (!self.runningWorkItemCount) {
// something is wrong with state - nothing is currently running
- os_unfair_lock_unlock(&_lock);
MTR_LOG_ERROR("MTRAsyncCallbackWorkQueue endWork: no work is running on work queue");
return;
}
@@ -129,7 +125,6 @@
MTRAsyncCallbackQueueWorkItem * firstWorkItem = self.items.firstObject;
if (firstWorkItem != workItem) {
// something is wrong with this work item - should not be currently running
- os_unfair_lock_unlock(&_lock);
MTR_LOG_ERROR("MTRAsyncCallbackWorkQueue endWork: work item is not first on work queue");
return;
}
@@ -142,7 +137,6 @@
// when "concurrency width" is implemented this will be decremented instead
self.runningWorkItemCount = 0;
[self _callNextReadyWorkItem];
- os_unfair_lock_unlock(&_lock);
}
- (void)endWork:(MTRAsyncCallbackQueueWorkItem *)workItem
@@ -203,34 +197,30 @@
- (void)invalidate
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
[self _invalidate];
- os_unfair_lock_unlock(&_lock);
}
- (void)markEnqueued
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
_enqueued = YES;
- os_unfair_lock_unlock(&_lock);
}
- (void)setReadyHandler:(MTRAsyncCallbackReadyHandler)readyHandler
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
if (!_enqueued) {
_readyHandler = readyHandler;
}
- os_unfair_lock_unlock(&_lock);
}
- (void)setCancelHandler:(dispatch_block_t)cancelHandler
{
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
if (!_enqueued) {
_cancelHandler = cancelHandler;
}
- os_unfair_lock_unlock(&_lock);
}
- (void)endWork
diff --git a/src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm b/src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm
index ff7242c..be83956 100644
--- a/src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm
+++ b/src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm
@@ -18,6 +18,7 @@
#import "MTRAsyncWorkQueue.h"
#import "MTRDefines_Internal.h"
#import "MTRLogging_Internal.h"
+#import "MTRUnfairLock.h"
#import <atomic>
#import <os/lock.h>
@@ -273,13 +274,12 @@
- (void)invalidate
{
ContextSnapshot context(_context); // outside of lock
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
MTR_LOG_INFO("MTRAsyncWorkQueue<%@> invalidate %tu items", context.description, _items.count);
for (MTRAsyncWorkItem * item in _items) {
[item cancel];
}
[_items removeAllObjects];
- os_unfair_lock_unlock(&_lock);
}
- (void)_postProcessWorkItem:(MTRAsyncWorkItem *)workItem
@@ -376,8 +376,7 @@
- (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData
{
- BOOL hasDuplicate = NO;
- os_unfair_lock_lock(&_lock);
+ std::lock_guard lock(_lock);
// Start from the last item
for (MTRAsyncWorkItem * item in [_items reverseObjectEnumerator]) {
auto duplicateCheckHandler = item.duplicateCheckHandler;
@@ -386,13 +385,12 @@
BOOL isDuplicate = NO;
duplicateCheckHandler(opaqueWorkItemData, &isDuplicate, &stop);
if (stop) {
- hasDuplicate = isDuplicate;
- break;
+ return isDuplicate;
}
}
}
- os_unfair_lock_unlock(&_lock);
- return hasDuplicate;
+
+ return NO;
}
@end
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index 98387bf..a63902f 100644
--- a/src/darwin/Framework/CHIP/MTRDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -352,22 +352,19 @@
- (void)_performScheduledTimeUpdate
{
- os_unfair_lock_lock(&self->_timeSyncLock);
+ std::lock_guard lock(_timeSyncLock);
// Device needs to still be reachable
if (self.state != MTRDeviceStateReachable) {
MTR_LOG_DEBUG("%@ Device is not reachable, canceling Device Time Updates.", self);
- os_unfair_lock_unlock(&self->_timeSyncLock);
return;
}
// Device must not be invalidated
if (!self.timeUpdateScheduled) {
MTR_LOG_DEBUG("%@ Device Time Update is no longer scheduled, MTRDevice may have been invalidated.", self);
- os_unfair_lock_unlock(&self->_timeSyncLock);
return;
}
self.timeUpdateScheduled = NO;
[self _updateDeviceTimeAndScheduleNextUpdate];
- os_unfair_lock_unlock(&self->_timeSyncLock);
}
- (NSArray<NSNumber *> *)_endpointsWithTimeSyncClusterServer
@@ -504,7 +501,7 @@
}
#endif
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
_weakDelegate = [MTRWeakReference weakReferenceWithObject:delegate];
_delegateQueue = queue;
@@ -517,8 +514,6 @@
if (setUpSubscription) {
[self _setupSubscription];
}
-
- os_unfair_lock_unlock(&self->_lock);
}
- (void)invalidate
@@ -579,11 +574,9 @@
// Return YES if there's a valid delegate AND subscription is expected to report value
- (BOOL)_subscriptionAbleToReport
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
id<MTRDeviceDelegate> delegate = _weakDelegate.strongObject;
- auto state = _state;
- os_unfair_lock_unlock(&self->_lock);
- return (delegate != nil) && (state == MTRDeviceStateReachable);
+ return (delegate != nil) && (_state == MTRDeviceStateReachable);
}
- (BOOL)_callDelegateWithBlock:(void (^)(id<MTRDeviceDelegate>))block
@@ -667,41 +660,35 @@
- (void)_handleSubscriptionError:(NSError *)error
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
_internalDeviceState = MTRInternalDeviceStateUnsubscribed;
_unreportedEvents = nil;
[self _changeState:MTRDeviceStateUnreachable];
-
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_handleResubscriptionNeeded
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
[self _changeState:MTRDeviceStateUnknown];
-
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_handleSubscriptionReset
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
// if there is no delegate then also do not retry
id<MTRDeviceDelegate> delegate = _weakDelegate.strongObject;
if (!delegate) {
// NOTE: Do not log anythig here: we have been invalidated, and the
// Matter stack might already be torn down.
- os_unfair_lock_unlock(&self->_lock);
return;
}
// don't schedule multiple retries
if (self.reattemptingSubscription) {
MTR_LOG_DEFAULT("%@ already reattempting subscription", self);
- os_unfair_lock_unlock(&self->_lock);
return;
}
@@ -722,8 +709,6 @@
[self _reattemptSubscriptionNowIfNeeded];
os_unfair_lock_unlock(&self->_lock);
});
-
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_reattemptSubscriptionNowIfNeeded
@@ -740,7 +725,7 @@
- (void)_handleUnsolicitedMessageFromPublisher
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
[self _changeState:MTRDeviceStateReachable];
@@ -759,8 +744,6 @@
// ReadClient how did we get this notification and if we _do_ have an active
// ReadClient, this call or _setupSubscription would be no-ops.
[self _reattemptSubscriptionNowIfNeeded];
-
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_markDeviceAsUnreachableIfNotSusbcribed
@@ -776,7 +759,8 @@
- (void)_handleReportBegin
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
+
_receivingReport = YES;
if (_state != MTRDeviceStateReachable) {
_receivingPrimingReport = YES;
@@ -784,12 +768,11 @@
} else {
_receivingPrimingReport = NO;
}
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_handleReportEnd
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
_receivingReport = NO;
_receivingPrimingReport = NO;
_estimatedStartTimeFromGeneralDiagnosticsUpTime = nil;
@@ -805,7 +788,6 @@
});
}
#endif
- os_unfair_lock_unlock(&self->_lock);
}
// assume lock is held
@@ -824,12 +806,10 @@
- (void)_handleAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
// _getAttributesToReportWithReportedValues will log attribute paths reported
[self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeReport]];
-
- os_unfair_lock_unlock(&self->_lock);
}
#ifdef DEBUG
@@ -843,7 +823,7 @@
- (void)_handleEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
NSDate * oldEstimatedStartTime = _estimatedStartTime;
// Combine with previous unreported events, if they exist
@@ -937,14 +917,13 @@
// save unreported events
_unreportedEvents = reportToReturn;
}
-
- os_unfair_lock_unlock(&self->_lock);
}
- (NSDictionary<MTRClusterPath *, NSNumber *> *)_getCachedDataVersions
{
NSMutableDictionary<MTRClusterPath *, NSNumber *> * dataVersions = [NSMutableDictionary dictionary];
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
+
for (MTRAttributePath * path in _readCache) {
NSDictionary * dataValue = _readCache[path];
NSNumber * dataVersionNumber = dataValue[MTRDataVersionKey];
@@ -958,7 +937,6 @@
}
}
MTR_LOG_INFO("%@ _getCachedDataVersions dataVersions count: %lu readCache count: %lu", self, static_cast<unsigned long>(dataVersions.count), static_cast<unsigned long>(_readCache.count));
- os_unfair_lock_unlock(&self->_lock);
return dataVersions;
}
@@ -1083,11 +1061,12 @@
os_unfair_lock_lock(&self->_lock);
self->_currentReadClient = nullptr;
self->_currentSubscriptionCallback = nullptr;
- os_unfair_lock_unlock(&self->_lock);
+
dispatch_async(self.queue, ^{
// OnDone
[self _handleSubscriptionReset];
});
+ os_unfair_lock_unlock(&self->_lock);
},
^(void) {
MTR_LOG_DEFAULT("%@ got unsolicited message from publisher", self);
@@ -1855,18 +1834,16 @@
- (void)_performScheduledExpirationCheck
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
self.expirationCheckScheduled = NO;
[self _checkExpiredExpectedValues];
-
- os_unfair_lock_unlock(&self->_lock);
}
// Get attribute value dictionary for an attribute path from the right cache
- (NSDictionary<NSString *, id> *)_attributeValueDictionaryForAttributePath:(MTRAttributePath *)attributePath
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
// First check expected value cache
NSArray * expectedValue = _expectedValueCache[attributePath];
@@ -1876,8 +1853,6 @@
// expired - purge and fall through
_expectedValueCache[attributePath] = nil;
} else {
- os_unfair_lock_unlock(&self->_lock);
-
// not yet expired - return result
return expectedValue[MTRDeviceExpectedValueFieldValueIndex];
}
@@ -1886,8 +1861,6 @@
// Then check read cache
NSDictionary<NSString *, id> * cachedAttributeValue = _readCache[attributePath];
if (cachedAttributeValue) {
- os_unfair_lock_unlock(&self->_lock);
-
return cachedAttributeValue;
} else {
// TODO: when not found in cache, generated default values should be used
@@ -1895,8 +1868,6 @@
attributePath);
}
- os_unfair_lock_unlock(&self->_lock);
-
return nil;
}
@@ -2024,7 +1995,7 @@
- (void)setAttributeValues:(NSArray<NSDictionary *> *)attributeValues reportChanges:(BOOL)reportChanges
{
MTR_LOG_INFO("%@ setAttributeValues count: %lu reportChanges: %d", self, static_cast<unsigned long>(attributeValues.count), reportChanges);
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
if (reportChanges) {
[self _reportAttributes:[self _getAttributesToReportWithReportedValues:attributeValues]];
@@ -2040,8 +2011,6 @@
if ([self _isCachePrimedWithInitialConfigurationData]) {
_delegateDeviceCachePrimedCalled = YES;
}
-
- os_unfair_lock_unlock(&self->_lock);
}
- (BOOL)deviceCachePrimed
@@ -2166,7 +2135,7 @@
MTR_LOG_INFO(
"%@ Setting expected values %@ with expiration time %f seconds from now", self, values, [expirationTime timeIntervalSinceNow]);
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
// _getAttributesToReportWithNewExpectedValues will log attribute paths reported
NSArray * attributesToReport = [self _getAttributesToReportWithNewExpectedValues:values
@@ -2175,24 +2144,22 @@
[self _reportAttributes:attributesToReport];
[self _checkExpiredExpectedValues];
- os_unfair_lock_unlock(&self->_lock);
}
- (void)removeExpectedValuesForAttributePaths:(NSArray<MTRAttributePath *> *)attributePaths
expectedValueID:(uint64_t)expectedValueID
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
+
for (MTRAttributePath * attributePath in attributePaths) {
[self _removeExpectedValueForAttributePath:attributePath expectedValueID:expectedValueID];
}
- os_unfair_lock_unlock(&self->_lock);
}
- (void)removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath expectedValueID:(uint64_t)expectedValueID
{
- os_unfair_lock_lock(&self->_lock);
+ std::lock_guard lock(_lock);
[self _removeExpectedValueForAttributePath:attributePath expectedValueID:expectedValueID];
- os_unfair_lock_unlock(&self->_lock);
}
- (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath expectedValueID:(uint64_t)expectedValueID
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
index 7b7fd45..ae6669c 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -46,6 +46,7 @@
#import "MTRServerEndpoint_Internal.h"
#import "MTRSetupPayload.h"
#import "MTRTimeUtils.h"
+#import "MTRUnfairLock.h"
#import "NSDataSpanConversion.h"
#import "NSStringSpanConversion.h"
#import <setup_payload/ManualSetupPayloadGenerator.h>
@@ -916,7 +917,7 @@
- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
{
- os_unfair_lock_lock(&_deviceMapLock);
+ std::lock_guard lock(_deviceMapLock);
MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID];
if (!deviceToReturn) {
deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
@@ -935,14 +936,13 @@
[deviceToReturn setAttributeValues:attributesFromCache reportChanges:NO];
}
}
- os_unfair_lock_unlock(&_deviceMapLock);
return deviceToReturn;
}
- (void)removeDevice:(MTRDevice *)device
{
- os_unfair_lock_lock(&_deviceMapLock);
+ std::lock_guard lock(_deviceMapLock);
auto * nodeID = device.nodeID;
MTRDevice * deviceToRemove = _nodeIDToDeviceMap[nodeID];
if (deviceToRemove == device) {
@@ -951,7 +951,6 @@
} else {
MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, nodeID.unsignedLongLongValue);
}
- os_unfair_lock_unlock(&_deviceMapLock);
}
- (void)setDeviceControllerDelegate:(id<MTRDeviceControllerDelegate>)delegate queue:(dispatch_queue_t)queue
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
index 2ccf896..3d84a8f 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
@@ -1121,10 +1121,8 @@
- (NSArray<MTRDeviceController *> *)getRunningControllers
{
- os_unfair_lock_lock(&_controllersLock);
- NSArray<MTRDeviceController *> * controllersCopy = [_controllers copy];
- os_unfair_lock_unlock(&_controllersLock);
- return controllersCopy;
+ std::lock_guard lock(_controllersLock);
+ return [_controllers copy];
}
- (nullable MTRDeviceController *)runningControllerForFabricIndex:(FabricIndex)fabricIndex
@@ -1196,9 +1194,8 @@
- (void)removeServerEndpoint:(MTRServerEndpoint *)endpoint
{
- os_unfair_lock_lock(&_serverEndpointsLock);
+ std::lock_guard lock(_serverEndpointsLock);
[_serverEndpoints removeObject:endpoint];
- os_unfair_lock_unlock(&_serverEndpointsLock);
}
- (NSArray<MTRAccessGrant *> *)accessGrantsForFabricIndex:(chip::FabricIndex)fabricIndex clusterPath:(MTRClusterPath *)clusterPath
diff --git a/src/darwin/Framework/CHIP/MTRLogging.mm b/src/darwin/Framework/CHIP/MTRLogging.mm
index 23b310a..e552978 100644
--- a/src/darwin/Framework/CHIP/MTRLogging.mm
+++ b/src/darwin/Framework/CHIP/MTRLogging.mm
@@ -19,6 +19,7 @@
#import "MTRLogging_Internal.h"
#import "MTRFramework.h"
+#import "MTRUnfairLock.h"
#import <algorithm>
#import <atomic>
@@ -56,7 +57,7 @@
{
MTRFrameworkInit();
- os_unfair_lock_lock(&logCallbackLock);
+ std::lock_guard lock(logCallbackLock);
if (callback) {
SetLogRedirectCallback(&MTRLogCallbackTrampoline);
SetLogFilter(static_cast<LogCategory>(std::min(std::max(logTypeThreshold, MTRLogTypeError), MTRLogTypeDetail)));
@@ -66,5 +67,4 @@
SetLogFilter(kLogCategory_None);
SetLogRedirectCallback(nullptr);
}
- os_unfair_lock_unlock(&logCallbackLock);
}
