From 910c152a4e66d7ec0dfe64562e554fa73845d879 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 06:24:17 -0700 Subject: [PATCH 01/11] [Darwin] Enable taking assertion on MTRDeviceController - Added ability to take assertion on MTRDeviceController to keep it running until all assertions are removed - A request to shutdown will take place only if there are no assertions are present --- .../Framework/CHIP/MTRDeviceController.mm | 56 +++++++++++++++++++ .../CHIP/MTRDeviceController_Concrete.mm | 10 ++++ .../CHIP/MTRDeviceController_Internal.h | 25 +++++++++ 3 files changed, 91 insertions(+) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 11fd481b48b2fc..3af017da66ca8a 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -142,6 +142,10 @@ - (instancetype)initForSubclasses // nothing, as superclass of MTRDeviceController is NSObject } _underlyingDeviceMapLock = OS_UNFAIR_LOCK_INIT; + + // Setup assertion counters + _keepRunningAssertionCounter = 0; + _shutdownPending = FALSE; return self; } @@ -178,6 +182,11 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory // Make sure our storage is all set up to work as early as possible, // before we start doing anything else with the controller. _uniqueIdentifier = uniqueIdentifier; + + // Setup assertion counters + _keepRunningAssertionCounter = 0; + _shutdownPending = FALSE; + if (storageDelegate != nil) { if (storageDelegateQueue == nil) { MTR_LOG_ERROR("storageDelegate provided without storageDelegateQueue"); @@ -311,7 +320,53 @@ - (BOOL)isRunning return _cppCommissioner != nullptr; } +- (NSUInteger)shutdownPrecheck +{ + __block NSUInteger assertionCount = 0; + dispatch_sync(_chipWorkQueue, ^{ + self.shutdownPending = TRUE; + assertionCount = self.keepRunningAssertionCounter; + }); + return assertionCount; +} + +- (void)addRunAssertion +{ + dispatch_sync(_chipWorkQueue, ^{ + ++self.keepRunningAssertionCounter; + MTR_LOG("%@ Adding keep running assertion, total %lu", self, self.keepRunningAssertionCounter); + }); +} + +- (void)removeRunAssertion; +{ + __block BOOL shouldShutdown = FALSE; + dispatch_sync(_chipWorkQueue, ^{ + if (self.keepRunningAssertionCounter > 0) { + --self.keepRunningAssertionCounter; + MTR_LOG("%@ Removing keep running assertion, total %lu", self, self.keepRunningAssertionCounter); + if (self.keepRunningAssertionCounter == 0 && self.shutdownPending) { + shouldShutdown = TRUE; + } + } + }); + if (shouldShutdown) { + MTR_LOG("%@ All assertions removed and shutdown is pending, shutting down", self); + [self finalShutdown]; + } +} + - (void)shutdown +{ + NSUInteger assertionCount = [self shutdownPrecheck]; + if (assertionCount != 0) { + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, assertionCount); + return; + } + [self finalShutdown]; +} + +- (void)finalShutdown { MTR_LOG("%@ shutdown called", self); if (_cppCommissioner == nullptr) { @@ -374,6 +429,7 @@ - (void)shutDownCppController _operationalCredentialsDelegate->SetDeviceCommissioner(nullptr); } } + self.shutdownPending = FALSE; } - (void)deinitFromFactory diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index 0ea7bf9232d766..d26d01bbc1cc0c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -326,6 +326,16 @@ - (BOOL)isRunning } - (void)shutdown +{ + NSUInteger assertionCount = [self shutdownPrecheck]; + if (assertionCount != 0) { + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, assertionCount); + return; + } + [self finalShutdown]; +} + +- (void)finalShutdown { MTR_LOG("%@ shutdown called", self); if (_cppCommissioner == nullptr) { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 54d5cfd8d340fa..29a694642dad02 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -72,6 +72,10 @@ NS_ASSUME_NONNULL_BEGIN // (moved here so subclasses can initialize differently) @property (readwrite, retain) dispatch_queue_t chipWorkQueue; +// Counters to track assertion status +@property (nonatomic, readwrite) NSUInteger keepRunningAssertionCounter; +@property (nonatomic, readwrite) BOOL shutdownPending; + - (instancetype)initForSubclasses; #pragma mark - MTRDeviceControllerFactory methods @@ -289,6 +293,27 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion; +/** + * Takes an assertion to keep the controller running. If `-[MTRDeviceController shutdown]` is called while an assertion + * is held, the shutdown will be honored only after all assertions are released. Invoking this method multiple times increases + * the number of assertions and needs to be matched with equal amount of '-[MTRDeviceController removeRunAssertion]` to release + * the assertion. + */ +- (void)addRunAssertion; + +/** + * Removes an assertion to allow the controller to shutdown once all assertions have been released. + * Invoking this method once all assertions have been released in a noop. + */ +- (void)removeRunAssertion; + +/** + * This methods marks a request to shutdown. + * Returns the number of run assertions currently being held. If the value returned is not zero, it implies shutdown has to be delayed + * until all assertions have been removed. + */ +- (NSUInteger)shutdownPrecheck; + @end /** From 0b3cf45fe928116793a0631152760c3770bb2ecc Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 08:24:11 -0700 Subject: [PATCH 02/11] Fixed format string --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 6 +++--- src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 3af017da66ca8a..8ff85e39f9c49c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -334,7 +334,7 @@ - (void)addRunAssertion { dispatch_sync(_chipWorkQueue, ^{ ++self.keepRunningAssertionCounter; - MTR_LOG("%@ Adding keep running assertion, total %lu", self, self.keepRunningAssertionCounter); + MTR_LOG("%@ Adding keep running assertion, total %lu", self, (unsigned long) self.keepRunningAssertionCounter); }); } @@ -344,7 +344,7 @@ - (void)removeRunAssertion; dispatch_sync(_chipWorkQueue, ^{ if (self.keepRunningAssertionCounter > 0) { --self.keepRunningAssertionCounter; - MTR_LOG("%@ Removing keep running assertion, total %lu", self, self.keepRunningAssertionCounter); + MTR_LOG("%@ Removing keep running assertion, total %lu", self, (unsigned long) self.keepRunningAssertionCounter); if (self.keepRunningAssertionCounter == 0 && self.shutdownPending) { shouldShutdown = TRUE; } @@ -360,7 +360,7 @@ - (void)shutdown { NSUInteger assertionCount = [self shutdownPrecheck]; if (assertionCount != 0) { - MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, assertionCount); + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) assertionCount); return; } [self finalShutdown]; diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index d26d01bbc1cc0c..dac43d1eec6403 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -329,7 +329,7 @@ - (void)shutdown { NSUInteger assertionCount = [self shutdownPrecheck]; if (assertionCount != 0) { - MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, assertionCount); + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) assertionCount); return; } [self finalShutdown]; From 7324f94de36e8bd4e76e0b4e2d91e12480610820 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 15:43:32 -0700 Subject: [PATCH 03/11] Account for existing controller that is asserted in factory when creating a new one --- .../Framework/CHIP/MTRDeviceController.mm | 48 +++++++++++++++++++ .../CHIP/MTRDeviceControllerFactory.mm | 20 ++++++++ .../CHIP/MTRDeviceControllerStartupParams.mm | 25 ++++++++++ ...TRDeviceControllerStartupParams_Internal.h | 4 ++ .../CHIP/MTRDeviceController_Internal.h | 11 +++++ 5 files changed, 108 insertions(+) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 8ff85e39f9c49c..b7c2e1395dd6ac 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -14,6 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include +#include +#include #import #import @@ -129,6 +132,10 @@ @implementation MTRDeviceController { std::atomic> _storedCompressedFabricID; MTRP256KeypairBridge _signingKeypairBridge; MTRP256KeypairBridge _operationalKeypairBridge; + + NSNumber * _fabricID; + NSNumber * _nodeID; + NSData * _rootPublicKey; } - (os_unfair_lock_t)deviceMapLock @@ -304,6 +311,9 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory _storedFabricIndex = chip::kUndefinedFabricIndex; _storedCompressedFabricID = std::nullopt; + _nodeID = nil; + _fabricID = nil; + _rootPublicKey = nil; _storageBehaviorConfiguration = storageBehaviorConfiguration; } @@ -330,6 +340,26 @@ - (NSUInteger)shutdownPrecheck return assertionCount; } +- (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parameters +{ + if (!parameters.operationalCertificate || !parameters.rootCertificate) { + return FALSE; + } + NSNumber *nodeID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; + NSNumber *fabricID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; + NSData *publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:parameters.rootCertificate]; + + __block BOOL matches = FALSE; + dispatch_sync(_chipWorkQueue, ^{ + matches = self.keepRunningAssertionCounter > 0 && + self.shutdownPending && + MTREqualObjects(nodeID, self->_nodeID) && + MTREqualObjects(fabricID, self->_fabricID) && + MTREqualObjects(publicKey, self->_rootPublicKey); + }); + return matches; +} + - (void)addRunAssertion { dispatch_sync(_chipWorkQueue, ^{ @@ -356,6 +386,13 @@ - (void)removeRunAssertion; } } +- (void)clearPendingShutdown +{ + dispatch_sync(_chipWorkQueue, ^{ + self.shutdownPending = FALSE; + }); +} + - (void)shutdown { NSUInteger assertionCount = [self shutdownPrecheck]; @@ -424,6 +461,9 @@ - (void)shutDownCppController // shuts down. _storedFabricIndex = chip::kUndefinedFabricIndex; _storedCompressedFabricID = std::nullopt; + _nodeID = nil; + _fabricID = nil; + _rootPublicKey = nil; delete commissionerToShutDown; if (_operationalCredentialsDelegate != nil) { _operationalCredentialsDelegate->SetDeviceCommissioner(nullptr); @@ -665,6 +705,14 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams self->_storedFabricIndex = fabricIdx; self->_storedCompressedFabricID = _cppCommissioner->GetCompressedFabricId(); + + chip::Crypto::P256PublicKey rootPublicKey; + if (_cppCommissioner->GetRootPublicKey(rootPublicKey) == CHIP_NO_ERROR) { + self->_rootPublicKey = [NSData dataWithBytes:rootPublicKey.Bytes() length:rootPublicKey.Length()]; + self->_nodeID = @(_cppCommissioner->GetNodeId()); + self->_fabricID = @(_cppCommissioner->GetFabricId()); + } + commissionerInitialized = YES; MTR_LOG("%@ startup succeeded for nodeID 0x%016llX", self, self->_cppCommissioner->GetNodeId()); diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 5b089b392074e6..ae2ca11efb2772 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -15,6 +15,7 @@ */ #import "MTRDeviceControllerFactory.h" +#include #import "MTRDeviceControllerFactory_Internal.h" #import @@ -1133,12 +1134,31 @@ - (void)operationalInstanceAdded:(chip::PeerId &)operationalID } } +- (nullable MTRDeviceController *)_findControllerMatchingParams:(MTRDeviceControllerParameters *)parameters +{ + std::lock_guard lock(_controllersLock); + for (MTRDeviceController *controller in _controllers) { + if ([controller matchesPendingShutdownWithParams:parameters]) { + MTR_LOG("%@ Found existing controller %@ that is pending shutdown and matching parameters, re-using it", self, controller); + [controller clearPendingShutdown]; + return controller; + } + } + return nil; +} + - (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)controller withParameters:(MTRDeviceControllerParameters *)parameters error:(NSError * __autoreleasing *)error { [self _assertCurrentQueueIsNotMatterQueue]; + // If there is a controller already running with matching parameters, re-use it + MTRDeviceController *existingController = [self _findControllerMatchingParams:parameters]; + if (existingController) { + return existingController; + } + return [self _startDeviceController:controller startupParams:parameters fabricChecker:^MTRDeviceControllerStartupParamsInternal *( diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index c5923ad4e54619..66792d4cedcf0f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -15,6 +15,8 @@ */ #import "MTRDeviceControllerStartupParams.h" +#include +#include #import "MTRCertificates.h" #import "MTRConversion.h" #import "MTRDeviceControllerStartupParams_Internal.h" @@ -306,6 +308,29 @@ - (void)setOTAProviderDelegate:(id)otaProviderDelegate q _otaProviderDelegateQueue = queue; } ++ (nullable NSNumber *)nodeIDFromNOC:(MTRCertificateDERBytes)noc +{ + NSNumber *nodeID = nil; + ExtractNodeIDFromNOC(noc, &nodeID); + return nodeID; +} + ++ (nullable NSNumber *)fabricIDFromNOC:(MTRCertificateDERBytes)noc +{ + NSNumber *fabricID = nil; + ExtractFabricIDFromNOC(noc, &fabricID); + return fabricID; +} + ++ (NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate +{ + Crypto::P256PublicKey pubKey; + if (ExtractPubkeyFromX509Cert(AsByteSpan(certificate), pubKey) != CHIP_NO_ERROR) { + return nil; + } + return [NSData dataWithBytes:pubKey.Bytes() length:pubKey.Length()]; +} + @end @implementation MTRDeviceControllerExternalCertificateParameters diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h index 6b8c762633578a..d5e35a63022a7c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h @@ -85,6 +85,10 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, strong, readonly, nullable) id otaProviderDelegate; @property (nonatomic, strong, readonly, nullable) dispatch_queue_t otaProviderDelegateQueue; ++ (nullable NSNumber *)nodeIDFromNOC:(MTRCertificateDERBytes)noc; ++ (nullable NSNumber *)fabricIDFromNOC:(MTRCertificateDERBytes)noc; ++ (NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate; + @end @interface MTRDeviceControllerStartupParamsInternal : MTRDeviceControllerStartupParams diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 29a694642dad02..5c4caa49cdd00f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -45,6 +45,7 @@ #import #import +@class MTRDeviceControllerParameters; @class MTRDeviceControllerStartupParamsInternal; @class MTRDeviceControllerFactory; @class MTRDevice; @@ -314,6 +315,16 @@ NS_ASSUME_NONNULL_BEGIN */ - (NSUInteger)shutdownPrecheck; +/** + * This method returns TRUE if this controller matches the fabric reference and node ID as listed in the parameters. + */ +- (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parameters; + +/** + * Clear any pending shutdown request. + */ +- (void)clearPendingShutdown; + @end /** From e2ed35bbc0a11ceb3dd00680a17f5026379f7d75 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 18:06:10 -0700 Subject: [PATCH 04/11] Simplified to use lock for assertion tracking --- .../Framework/CHIP/MTRDeviceController.mm | 77 +++++++++---------- .../CHIP/MTRDeviceController_Internal.h | 4 - 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index b7c2e1395dd6ac..d87a6c3c210326 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -136,6 +136,11 @@ @implementation MTRDeviceController { NSNumber * _fabricID; NSNumber * _nodeID; NSData * _rootPublicKey; + + // Counters to track assertion status and access controlled by the _assertionLock + NSUInteger _keepRunningAssertionCounter; + BOOL _shutdownPending; + os_unfair_lock _assertionLock; } - (os_unfair_lock_t)deviceMapLock @@ -150,9 +155,10 @@ - (instancetype)initForSubclasses } _underlyingDeviceMapLock = OS_UNFAIR_LOCK_INIT; - // Setup assertion counters + // Setup assertion variables _keepRunningAssertionCounter = 0; - _shutdownPending = FALSE; + _shutdownPending = NO; + _assertionLock = OS_UNFAIR_LOCK_INIT; return self; } @@ -190,9 +196,10 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory // before we start doing anything else with the controller. _uniqueIdentifier = uniqueIdentifier; - // Setup assertion counters + // Setup assertion variables _keepRunningAssertionCounter = 0; - _shutdownPending = FALSE; + _shutdownPending = NO; + _assertionLock = OS_UNFAIR_LOCK_INIT; if (storageDelegate != nil) { if (storageDelegateQueue == nil) { @@ -330,16 +337,6 @@ - (BOOL)isRunning return _cppCommissioner != nullptr; } -- (NSUInteger)shutdownPrecheck -{ - __block NSUInteger assertionCount = 0; - dispatch_sync(_chipWorkQueue, ^{ - self.shutdownPending = TRUE; - assertionCount = self.keepRunningAssertionCounter; - }); - return assertionCount; -} - - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parameters { if (!parameters.operationalCertificate || !parameters.rootCertificate) { @@ -351,8 +348,8 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame __block BOOL matches = FALSE; dispatch_sync(_chipWorkQueue, ^{ - matches = self.keepRunningAssertionCounter > 0 && - self.shutdownPending && + matches = _keepRunningAssertionCounter > 0 && + _shutdownPending && MTREqualObjects(nodeID, self->_nodeID) && MTREqualObjects(fabricID, self->_fabricID) && MTREqualObjects(publicKey, self->_rootPublicKey); @@ -362,42 +359,42 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame - (void)addRunAssertion { - dispatch_sync(_chipWorkQueue, ^{ - ++self.keepRunningAssertionCounter; - MTR_LOG("%@ Adding keep running assertion, total %lu", self, (unsigned long) self.keepRunningAssertionCounter); - }); + std::lock_guard lock(_assertionLock); + + // Only take an assertion if running + if ([self isRunning]) { + ++_keepRunningAssertionCounter; + MTR_LOG("%@ Adding keep running assertion, total %lu", self, (unsigned long) _keepRunningAssertionCounter); + } } - (void)removeRunAssertion; { - __block BOOL shouldShutdown = FALSE; - dispatch_sync(_chipWorkQueue, ^{ - if (self.keepRunningAssertionCounter > 0) { - --self.keepRunningAssertionCounter; - MTR_LOG("%@ Removing keep running assertion, total %lu", self, (unsigned long) self.keepRunningAssertionCounter); - if (self.keepRunningAssertionCounter == 0 && self.shutdownPending) { - shouldShutdown = TRUE; - } + std::lock_guard lock(_assertionLock); + + if (_keepRunningAssertionCounter > 0) { + --_keepRunningAssertionCounter; + MTR_LOG("%@ Removing keep running assertion, total %lu", self, (unsigned long) _keepRunningAssertionCounter); + + if ([self isRunning] && _keepRunningAssertionCounter == 0 && _shutdownPending) { + MTR_LOG("%@ All assertions removed and shutdown is pending, shutting down", self); + [self finalShutdown]; } - }); - if (shouldShutdown) { - MTR_LOG("%@ All assertions removed and shutdown is pending, shutting down", self); - [self finalShutdown]; } } - (void)clearPendingShutdown { - dispatch_sync(_chipWorkQueue, ^{ - self.shutdownPending = FALSE; - }); + std::lock_guard lock(_assertionLock); + _shutdownPending = NO; } - (void)shutdown { - NSUInteger assertionCount = [self shutdownPrecheck]; - if (assertionCount != 0) { - MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) assertionCount); + std::lock_guard lock(_assertionLock); + + if (_keepRunningAssertionCounter > 0) { + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) _keepRunningAssertionCounter); return; } [self finalShutdown]; @@ -405,6 +402,8 @@ - (void)shutdown - (void)finalShutdown { + os_unfair_lock_assert_owner(&_assertionLock); + MTR_LOG("%@ shutdown called", self); if (_cppCommissioner == nullptr) { // Already shut down. @@ -469,7 +468,7 @@ - (void)shutDownCppController _operationalCredentialsDelegate->SetDeviceCommissioner(nullptr); } } - self.shutdownPending = FALSE; + _shutdownPending = NO; } - (void)deinitFromFactory diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 5c4caa49cdd00f..37392c02bb5eda 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -73,10 +73,6 @@ NS_ASSUME_NONNULL_BEGIN // (moved here so subclasses can initialize differently) @property (readwrite, retain) dispatch_queue_t chipWorkQueue; -// Counters to track assertion status -@property (nonatomic, readwrite) NSUInteger keepRunningAssertionCounter; -@property (nonatomic, readwrite) BOOL shutdownPending; - - (instancetype)initForSubclasses; #pragma mark - MTRDeviceControllerFactory methods From b48b28832389f684604e2b796e8c66d9b080f901 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 18:12:22 -0700 Subject: [PATCH 05/11] Fixed build error --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 16 +++++++--------- .../CHIP/MTRDeviceController_Concrete.mm | 10 ---------- .../CHIP/MTRDeviceController_Internal.h | 7 ------- 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index d87a6c3c210326..8115b0de987d84 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -346,15 +346,13 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame NSNumber *fabricID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; NSData *publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:parameters.rootCertificate]; - __block BOOL matches = FALSE; - dispatch_sync(_chipWorkQueue, ^{ - matches = _keepRunningAssertionCounter > 0 && - _shutdownPending && - MTREqualObjects(nodeID, self->_nodeID) && - MTREqualObjects(fabricID, self->_fabricID) && - MTREqualObjects(publicKey, self->_rootPublicKey); - }); - return matches; + std::lock_guard lock(_assertionLock); + + return _keepRunningAssertionCounter > 0 && + _shutdownPending && + MTREqualObjects(nodeID, self->_nodeID) && + MTREqualObjects(fabricID, self->_fabricID) && + MTREqualObjects(publicKey, self->_rootPublicKey); } - (void)addRunAssertion diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index dac43d1eec6403..0ea7bf9232d766 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -326,16 +326,6 @@ - (BOOL)isRunning } - (void)shutdown -{ - NSUInteger assertionCount = [self shutdownPrecheck]; - if (assertionCount != 0) { - MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) assertionCount); - return; - } - [self finalShutdown]; -} - -- (void)finalShutdown { MTR_LOG("%@ shutdown called", self); if (_cppCommissioner == nullptr) { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 37392c02bb5eda..7b3d4b4974ef7a 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -304,13 +304,6 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)removeRunAssertion; -/** - * This methods marks a request to shutdown. - * Returns the number of run assertions currently being held. If the value returned is not zero, it implies shutdown has to be delayed - * until all assertions have been removed. - */ -- (NSUInteger)shutdownPrecheck; - /** * This method returns TRUE if this controller matches the fabric reference and node ID as listed in the parameters. */ From 198eaae64395b7321f31cd31eef8a1ad7bbc1bf8 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 18:49:30 -0700 Subject: [PATCH 06/11] Removed unneeded includes --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 4 +--- src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm | 1 - src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm | 2 -- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 8115b0de987d84..a338dcfa289655 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -14,9 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include -#include -#include #import #import @@ -49,6 +46,7 @@ #import "MTRSetupPayload.h" #import "MTRTimeUtils.h" #import "MTRUnfairLock.h" +#import "MTRUtilities.h" #import "NSDataSpanConversion.h" #import "NSStringSpanConversion.h" #import diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index ae2ca11efb2772..80e1c216eb90a3 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -15,7 +15,6 @@ */ #import "MTRDeviceControllerFactory.h" -#include #import "MTRDeviceControllerFactory_Internal.h" #import diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index 66792d4cedcf0f..55e0f1ee2d04e6 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -15,8 +15,6 @@ */ #import "MTRDeviceControllerStartupParams.h" -#include -#include #import "MTRCertificates.h" #import "MTRConversion.h" #import "MTRDeviceControllerStartupParams_Internal.h" From 9c0fe0aa3336b3e38628afae6e3ed41153055f3e Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 22:26:17 -0700 Subject: [PATCH 07/11] Fixed bugs with wrong match logic --- .../Framework/CHIP/MTRDeviceController.mm | 32 +++++++++---------- .../CHIP/MTRDeviceController_Internal.h | 16 ++++++++++ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index a338dcfa289655..c222593369b583 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -131,10 +131,6 @@ @implementation MTRDeviceController { MTRP256KeypairBridge _signingKeypairBridge; MTRP256KeypairBridge _operationalKeypairBridge; - NSNumber * _fabricID; - NSNumber * _nodeID; - NSData * _rootPublicKey; - // Counters to track assertion status and access controlled by the _assertionLock NSUInteger _keepRunningAssertionCounter; BOOL _shutdownPending; @@ -316,9 +312,9 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory _storedFabricIndex = chip::kUndefinedFabricIndex; _storedCompressedFabricID = std::nullopt; - _nodeID = nil; - _fabricID = nil; - _rootPublicKey = nil; + self.nodeID = nil; + self.fabricID = nil; + self.rootPublicKey = nil; _storageBehaviorConfiguration = storageBehaviorConfiguration; } @@ -341,16 +337,16 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame return FALSE; } NSNumber *nodeID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; - NSNumber *fabricID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; + NSNumber *fabricID = [MTRDeviceControllerParameters fabricIDFromNOC:parameters.operationalCertificate]; NSData *publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:parameters.rootCertificate]; std::lock_guard lock(_assertionLock); return _keepRunningAssertionCounter > 0 && _shutdownPending && - MTREqualObjects(nodeID, self->_nodeID) && - MTREqualObjects(fabricID, self->_fabricID) && - MTREqualObjects(publicKey, self->_rootPublicKey); + MTREqualObjects(nodeID, self.nodeID) && + MTREqualObjects(fabricID, self.fabricID) && + MTREqualObjects(publicKey, self.rootPublicKey); } - (void)addRunAssertion @@ -391,6 +387,7 @@ - (void)shutdown if (_keepRunningAssertionCounter > 0) { MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) _keepRunningAssertionCounter); + _shutdownPending = YES; return; } [self finalShutdown]; @@ -456,9 +453,10 @@ - (void)shutDownCppController // shuts down. _storedFabricIndex = chip::kUndefinedFabricIndex; _storedCompressedFabricID = std::nullopt; - _nodeID = nil; - _fabricID = nil; - _rootPublicKey = nil; + self.nodeID = nil; + self.fabricID = nil; + self.rootPublicKey = nil; + delete commissionerToShutDown; if (_operationalCredentialsDelegate != nil) { _operationalCredentialsDelegate->SetDeviceCommissioner(nullptr); @@ -703,9 +701,9 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams chip::Crypto::P256PublicKey rootPublicKey; if (_cppCommissioner->GetRootPublicKey(rootPublicKey) == CHIP_NO_ERROR) { - self->_rootPublicKey = [NSData dataWithBytes:rootPublicKey.Bytes() length:rootPublicKey.Length()]; - self->_nodeID = @(_cppCommissioner->GetNodeId()); - self->_fabricID = @(_cppCommissioner->GetFabricId()); + self.rootPublicKey = [NSData dataWithBytes:rootPublicKey.Bytes() length:rootPublicKey.Length()]; + self.nodeID = @(_cppCommissioner->GetNodeId()); + self.fabricID = @(_cppCommissioner->GetFabricId()); } commissionerInitialized = YES; diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 7b3d4b4974ef7a..e08569e5a9416d 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -118,6 +118,22 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic, readonly) MTRAsyncWorkQueue * concurrentSubscriptionPool; +/** + * Fabric ID tied to controller + */ +@property (nonatomic, retain, nullable) NSNumber * fabricID; + +/** + * Node ID tied to controller + */ +@property (nonatomic, retain, nullable) NSNumber * nodeID; + +/** + * Root Public Key tied to controller + */ +@property (nonatomic, retain, nullable) NSData * rootPublicKey; + + /** * Init a newly created controller. * From 35d220ee91e1149d68033a10a9334413f1a8fb24 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Thu, 22 Aug 2024 22:27:40 -0700 Subject: [PATCH 08/11] resytle fixes --- src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm | 4 ++-- src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm | 4 ++-- src/darwin/Framework/CHIP/MTRDeviceController_Internal.h | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 80e1c216eb90a3..acc1933a4cee84 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1136,7 +1136,7 @@ - (void)operationalInstanceAdded:(chip::PeerId &)operationalID - (nullable MTRDeviceController *)_findControllerMatchingParams:(MTRDeviceControllerParameters *)parameters { std::lock_guard lock(_controllersLock); - for (MTRDeviceController *controller in _controllers) { + for (MTRDeviceController * controller in _controllers) { if ([controller matchesPendingShutdownWithParams:parameters]) { MTR_LOG("%@ Found existing controller %@ that is pending shutdown and matching parameters, re-using it", self, controller); [controller clearPendingShutdown]; @@ -1153,7 +1153,7 @@ - (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)co [self _assertCurrentQueueIsNotMatterQueue]; // If there is a controller already running with matching parameters, re-use it - MTRDeviceController *existingController = [self _findControllerMatchingParams:parameters]; + MTRDeviceController * existingController = [self _findControllerMatchingParams:parameters]; if (existingController) { return existingController; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index 55e0f1ee2d04e6..06deae1bd9bad7 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -308,14 +308,14 @@ - (void)setOTAProviderDelegate:(id)otaProviderDelegate q + (nullable NSNumber *)nodeIDFromNOC:(MTRCertificateDERBytes)noc { - NSNumber *nodeID = nil; + NSNumber * nodeID = nil; ExtractNodeIDFromNOC(noc, &nodeID); return nodeID; } + (nullable NSNumber *)fabricIDFromNOC:(MTRCertificateDERBytes)noc { - NSNumber *fabricID = nil; + NSNumber * fabricID = nil; ExtractFabricIDFromNOC(noc, &fabricID); return fabricID; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index e08569e5a9416d..e625d13a80e77e 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -133,7 +133,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic, retain, nullable) NSData * rootPublicKey; - /** * Init a newly created controller. * From 7ba72a65c02ab8a22fd37301e607d8d257b4bc75 Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Fri, 23 Aug 2024 09:40:50 -0700 Subject: [PATCH 09/11] Restyle fixes --- .../Framework/CHIP/MTRDeviceController.mm | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index c222593369b583..cf6aa8a1871fdf 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -336,17 +336,13 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame if (!parameters.operationalCertificate || !parameters.rootCertificate) { return FALSE; } - NSNumber *nodeID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; - NSNumber *fabricID = [MTRDeviceControllerParameters fabricIDFromNOC:parameters.operationalCertificate]; - NSData *publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:parameters.rootCertificate]; + NSNumber * nodeID = [MTRDeviceControllerParameters nodeIDFromNOC:parameters.operationalCertificate]; + NSNumber * fabricID = [MTRDeviceControllerParameters fabricIDFromNOC:parameters.operationalCertificate]; + NSData * publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:parameters.rootCertificate]; std::lock_guard lock(_assertionLock); - return _keepRunningAssertionCounter > 0 && - _shutdownPending && - MTREqualObjects(nodeID, self.nodeID) && - MTREqualObjects(fabricID, self.fabricID) && - MTREqualObjects(publicKey, self.rootPublicKey); + return _keepRunningAssertionCounter > 0 && _shutdownPending && MTREqualObjects(nodeID, self.nodeID) && MTREqualObjects(fabricID, self.fabricID) && MTREqualObjects(publicKey, self.rootPublicKey); } - (void)addRunAssertion @@ -701,9 +697,9 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams chip::Crypto::P256PublicKey rootPublicKey; if (_cppCommissioner->GetRootPublicKey(rootPublicKey) == CHIP_NO_ERROR) { - self.rootPublicKey = [NSData dataWithBytes:rootPublicKey.Bytes() length:rootPublicKey.Length()]; - self.nodeID = @(_cppCommissioner->GetNodeId()); - self.fabricID = @(_cppCommissioner->GetFabricId()); + self.rootPublicKey = [NSData dataWithBytes:rootPublicKey.Bytes() length:rootPublicKey.Length()]; + self.nodeID = @(_cppCommissioner->GetNodeId()); + self.fabricID = @(_cppCommissioner->GetFabricId()); } commissionerInitialized = YES; From 0a89d13d0a2181f9fb1c62992ad9faf37b5a2d9b Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Fri, 23 Aug 2024 10:07:14 -0700 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 6 +++--- src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm | 4 ++-- .../Framework/CHIP/MTRDeviceControllerStartupParams.mm | 2 +- .../CHIP/MTRDeviceControllerStartupParams_Internal.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index cf6aa8a1871fdf..5e832bc082c966 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -352,7 +352,7 @@ - (void)addRunAssertion // Only take an assertion if running if ([self isRunning]) { ++_keepRunningAssertionCounter; - MTR_LOG("%@ Adding keep running assertion, total %lu", self, (unsigned long) _keepRunningAssertionCounter); + MTR_LOG("%@ Adding keep running assertion, total %lu", self, static_cast(_keepRunningAssertionCounter)); } } @@ -362,7 +362,7 @@ - (void)removeRunAssertion; if (_keepRunningAssertionCounter > 0) { --_keepRunningAssertionCounter; - MTR_LOG("%@ Removing keep running assertion, total %lu", self, (unsigned long) _keepRunningAssertionCounter); + MTR_LOG("%@ Removing keep running assertion, total %lu", self, static_cast(_keepRunningAssertionCounter)); if ([self isRunning] && _keepRunningAssertionCounter == 0 && _shutdownPending) { MTR_LOG("%@ All assertions removed and shutdown is pending, shutting down", self); @@ -382,7 +382,7 @@ - (void)shutdown std::lock_guard lock(_assertionLock); if (_keepRunningAssertionCounter > 0) { - MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, (unsigned long) _keepRunningAssertionCounter); + MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, static_cast(_keepRunningAssertionCounter)); _shutdownPending = YES; return; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index acc1933a4cee84..f7297d6804e494 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1133,7 +1133,7 @@ - (void)operationalInstanceAdded:(chip::PeerId &)operationalID } } -- (nullable MTRDeviceController *)_findControllerMatchingParams:(MTRDeviceControllerParameters *)parameters +- (nullable MTRDeviceController *)_findControllerWithPendingShutdownMatchingParams:(MTRDeviceControllerParameters *)parameters { std::lock_guard lock(_controllersLock); for (MTRDeviceController * controller in _controllers) { @@ -1152,7 +1152,7 @@ - (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)co { [self _assertCurrentQueueIsNotMatterQueue]; - // If there is a controller already running with matching parameters, re-use it + // If there is a controller already running with matching parameters that is conceptually shut down from the API consumer's viewpoint, re-use it. MTRDeviceController * existingController = [self _findControllerMatchingParams:parameters]; if (existingController) { return existingController; diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index 06deae1bd9bad7..5850a8c4efe384 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -320,7 +320,7 @@ + (nullable NSNumber *)fabricIDFromNOC:(MTRCertificateDERBytes)noc return fabricID; } -+ (NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate ++ (nullable NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate { Crypto::P256PublicKey pubKey; if (ExtractPubkeyFromX509Cert(AsByteSpan(certificate), pubKey) != CHIP_NO_ERROR) { diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h index d5e35a63022a7c..0b4065f491873b 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams_Internal.h @@ -87,7 +87,7 @@ NS_ASSUME_NONNULL_BEGIN + (nullable NSNumber *)nodeIDFromNOC:(MTRCertificateDERBytes)noc; + (nullable NSNumber *)fabricIDFromNOC:(MTRCertificateDERBytes)noc; -+ (NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate; ++ (nullable NSData *)publicKeyFromCertificate:(MTRCertificateDERBytes)certificate; @end From f8ea1231ca4313e3149b863e2628234648c4e91c Mon Sep 17 00:00:00 2001 From: Anush Nadathur Date: Fri, 23 Aug 2024 10:14:44 -0700 Subject: [PATCH 11/11] Fixed build failure from review suggestions and added comment per review feedback --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 1 + src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 5e832bc082c966..4630ff7fe00b4f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -342,6 +342,7 @@ - (BOOL)matchesPendingShutdownWithParams:(MTRDeviceControllerParameters *)parame std::lock_guard lock(_assertionLock); + // If any of the local above are nil, the return will be false since MTREqualObjects handles them correctly return _keepRunningAssertionCounter > 0 && _shutdownPending && MTREqualObjects(nodeID, self.nodeID) && MTREqualObjects(fabricID, self.fabricID) && MTREqualObjects(publicKey, self.rootPublicKey); } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index f7297d6804e494..bd0b90450b9ac4 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1153,7 +1153,7 @@ - (nullable MTRDeviceController *)initializeController:(MTRDeviceController *)co [self _assertCurrentQueueIsNotMatterQueue]; // If there is a controller already running with matching parameters that is conceptually shut down from the API consumer's viewpoint, re-use it. - MTRDeviceController * existingController = [self _findControllerMatchingParams:parameters]; + MTRDeviceController * existingController = [self _findControllerWithPendingShutdownMatchingParams:parameters]; if (existingController) { return existingController; }