From 4606595fcd58e724312cadd83697f3589a780d83 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 14 Dec 2022 14:13:32 -0500 Subject: [PATCH] Make some more safety improvements to dispatch to the Matter queue. (#24084) The idea is to minimize the number of places that have to have isRunning checks, and direct access to the Matter queue, by having APIs that do those for you. --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 6 +- .../CHIP/MTRCallbackBridgeBase_internal.h | 31 ++-- .../CHIP/MTRClusterStateCacheContainer.mm | 2 +- .../Framework/CHIP/MTRDeviceController.mm | 158 ++++++++---------- .../CHIP/MTRDeviceController_Internal.h | 62 ++++--- .../CHIP/MTROTAProviderDelegateBridge.mm | 10 +- .../CHIP/MTROperationalCredentialsDelegate.h | 3 - .../CHIP/MTROperationalCredentialsDelegate.mm | 96 +++++------ 8 files changed, 183 insertions(+), 185 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index eb39a8609f7350..752d14c9c19837 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -137,7 +137,7 @@ static void PurgeReadClientContainers( // Destroy read clients in the work queue [controller - asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner * commissioner) { + asyncDispatchToMatterQueue:^() { for (MTRReadClientContainer * container in listToDelete) { if (container.readClientPtr) { Platform::Delete(container.readClientPtr); @@ -193,7 +193,7 @@ static void CauseReadClientFailure( [readClientContainersLock unlock]; [controller - asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner * commissioner) { + asyncDispatchToMatterQueue:^() { for (MTRReadClientContainer * container in listToFail) { // Send auto resubscribe request again by read clients, which must fail. chip::app::ReadPrepareParams readParams; @@ -1373,7 +1373,7 @@ - (void)openCommissioningWindowWithSetupPasscode:(NSNumber *)setupPasscode } [self.deviceController - asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner * commissioner) { + asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) { auto resultCallback = ^(CHIP_ERROR status, const SetupPayload & payload) { if (status != CHIP_NO_ERROR) { dispatch_async(queue, ^{ diff --git a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h index c6ef366925302f..4304f15cd52a34 100644 --- a/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h +++ b/src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h @@ -139,7 +139,7 @@ template class MTRCallbackBridge : public MTRCallbackBridgeBase { LogRequestStart(); [device.deviceController - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { CHIP_ERROR err = action(mSuccess, mFailure); if (err != CHIP_NO_ERROR) { NSLog(@"Failure performing action. C++-mangled success callback type: '%s', error: %s", typeid(T).name(), @@ -159,31 +159,22 @@ template class MTRCallbackBridge : public MTRCallbackBridgeBase { { LogRequestStart(); - BOOL ok = [device.deviceController - getSessionForCommissioneeDevice:device.nodeID - completion:^(chip::Messaging::ExchangeManager * exchangeManager, - const chip::Optional & session, NSError * error) { - MaybeDoAction(exchangeManager, session, error); - }]; - - if (ok == NO) { - OnFailureFn(this, CHIP_ERROR_INCORRECT_STATE); - } + [device.deviceController getSessionForCommissioneeDevice:device.nodeID + completion:^(chip::Messaging::ExchangeManager * exchangeManager, + const chip::Optional & session, NSError * error) { + MaybeDoAction(exchangeManager, session, error); + }]; } void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController * controller) { LogRequestStart(); - BOOL ok = [controller getSessionForNode:nodeID - completion:^(chip::Messaging::ExchangeManager * exchangeManager, - const chip::Optional & session, NSError * error) { - MaybeDoAction(exchangeManager, session, error); - }]; - - if (ok == NO) { - OnFailureFn(this, CHIP_ERROR_INCORRECT_STATE); - } + [controller getSessionForNode:nodeID + completion:^(chip::Messaging::ExchangeManager * exchangeManager, + const chip::Optional & session, NSError * error) { + MaybeDoAction(exchangeManager, session, error); + }]; } void LogRequestStart() diff --git a/src/darwin/Framework/CHIP/MTRClusterStateCacheContainer.mm b/src/darwin/Framework/CHIP/MTRClusterStateCacheContainer.mm index 7550e1ef3f2448..1027d0081af473 100644 --- a/src/darwin/Framework/CHIP/MTRClusterStateCacheContainer.mm +++ b/src/darwin/Framework/CHIP/MTRClusterStateCacheContainer.mm @@ -137,7 +137,7 @@ - (void)readAttributesWithEndpointID:(NSNumber * _Nullable)endpointID } [self.baseDevice.deviceController - asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { if (endpointID == nil && clusterID == nil) { MTR_LOG_ERROR( "Error: currently read from attribute cache does not support wildcards for both endpoint and cluster"); diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index f4d648035af244..0fa42fe57eb5ef 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -123,7 +123,6 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory queue:(dis if ([self checkForInitError:(_operationalCredentialsDelegate != nullptr) logMsg:kErrorOperationalCredentialsInit]) { return nil; } - _operationalCredentialsDelegate->setChipWorkQueue(_chipWorkQueue); } return self; } @@ -544,13 +543,11 @@ - (void)removeDevice:(MTRDevice *)device - (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue { - VerifyOrReturn([self checkIsRunning]); - - dispatch_async(_chipWorkQueue, ^{ - VerifyOrReturn([self checkIsRunning]); - - self->_deviceControllerDelegateBridge->setDelegate(self, delegate, queue); - }); + [self + asyncDispatchToMatterQueue:^() { + self->_deviceControllerDelegateBridge->setDelegate(self, delegate, queue); + } + errorHandler:nil]; } - (BOOL)setOperationalCertificateIssuer:(nullable id)operationalCertificateIssuer @@ -692,68 +689,54 @@ - (BOOL)_deviceBeingCommissionedOverBLE:(uint64_t)deviceID return deviceProxy->GetDeviceTransportType() == chip::Transport::Type::kBle; } -- (BOOL)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion +- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion { - if (![self checkIsRunning]) { - return NO; - } + [self + asyncGetCommissionerOnMatterQueue:^(chip::Controller::DeviceCommissioner * commissioner) { + auto connectionBridge = new MTRDeviceConnectionBridge(completion); - dispatch_async(_chipWorkQueue, ^{ - NSError * error; - if (![self checkIsRunning:&error]) { - completion(nullptr, chip::NullOptional, error); - return; + // MTRDeviceConnectionBridge always delivers errors async via + // completion. + connectionBridge->connect(commissioner, nodeID); } - - auto connectionBridge = new MTRDeviceConnectionBridge(completion); - - // MTRDeviceConnectionBridge always delivers errors async via - // completion. - connectionBridge->connect(self->_cppCommissioner, nodeID); - }); - - return YES; + errorHandler:^(NSError * error) { + completion(nullptr, chip::NullOptional, error); + }]; } -- (BOOL)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion +- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion { - if (![self checkIsRunning]) { - return NO; - } - - dispatch_async(_chipWorkQueue, ^{ - NSError * error; - if (![self checkIsRunning:&error]) { - completion(nullptr, chip::NullOptional, error); - return; - } + [self + asyncGetCommissionerOnMatterQueue:^(chip::Controller::DeviceCommissioner * commissioner) { + chip::CommissioneeDeviceProxy * deviceProxy; + CHIP_ERROR err = commissioner->GetDeviceBeingCommissioned(deviceID, &deviceProxy); + if (err != CHIP_NO_ERROR) { + completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err]); + return; + } - chip::CommissioneeDeviceProxy * deviceProxy; - CHIP_ERROR err = self->_cppCommissioner->GetDeviceBeingCommissioned(deviceID, &deviceProxy); - if (err != CHIP_NO_ERROR) { - completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err]); - return; - } + chip::Optional session = deviceProxy->GetSecureSession(); + if (!session.HasValue() || !session.Value()->AsSecureSession()->IsPASESession()) { + completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); + return; + } - chip::Optional session = deviceProxy->GetSecureSession(); - if (!session.HasValue() || !session.Value()->AsSecureSession()->IsPASESession()) { - completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]); - return; + completion(deviceProxy->GetExchangeManager(), session, nil); } - - completion(deviceProxy->GetExchangeManager(), session, nil); - }); - - return YES; + errorHandler:^(NSError * error) { + completion(nullptr, chip::NullOptional, error); + }]; } -- (void)asyncDispatchToMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block - errorHandler:(void (^)(NSError *))errorHandler +- (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block + errorHandler:(nullable MTRDeviceErrorHandler)errorHandler { { NSError * error; if (![self checkIsRunning:&error]) { - errorHandler(error); + if (errorHandler != nil) { + errorHandler(error); + } return; } } @@ -761,7 +744,9 @@ - (void)asyncDispatchToMatterQueue:(void (^)(chip::Controller::DeviceCommissione dispatch_async(_chipWorkQueue, ^{ NSError * error; if (![self checkIsRunning:&error]) { - errorHandler(error); + if (errorHandler != nil) { + errorHandler(error); + } return; } @@ -769,6 +754,14 @@ - (void)asyncDispatchToMatterQueue:(void (^)(chip::Controller::DeviceCommissione }); } +- (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler +{ + auto adapter = ^(chip::Controller::DeviceCommissioner *) { + block(); + }; + [self asyncGetCommissionerOnMatterQueue:adapter errorHandler:errorHandler]; +} + - (void)syncRunOnWorkQueue:(SyncWorkQueueBlock)block error:(NSError * __autoreleasing *)error { VerifyOrReturn([self checkIsRunning:error]); @@ -782,13 +775,11 @@ - (void)syncRunOnWorkQueue:(SyncWorkQueueBlock)block error:(NSError * __autorele - (id)syncRunOnWorkQueueWithReturnValue:(SyncWorkQueueBlockWithReturnValue)block error:(NSError * __autoreleasing *)error { __block id rv = nil; - - VerifyOrReturnValue([self checkIsRunning:error], rv); - - dispatch_sync(_chipWorkQueue, ^{ - VerifyOrReturn([self checkIsRunning:error]); + auto adapter = ^{ rv = block(); - }); + }; + + [self syncRunOnWorkQueue:adapter error:error]; return rv; } @@ -796,13 +787,10 @@ - (id)syncRunOnWorkQueueWithReturnValue:(SyncWorkQueueBlockWithReturnValue)block - (BOOL)syncRunOnWorkQueueWithBoolReturnValue:(SyncWorkQueueBlockWithBoolReturnValue)block error:(NSError * __autoreleasing *)error { __block BOOL success = NO; - - VerifyOrReturnValue([self checkIsRunning:error], success); - - dispatch_sync(_chipWorkQueue, ^{ - VerifyOrReturn([self checkIsRunning:error]); + auto adapter = ^{ success = block(); - }); + }; + [self syncRunOnWorkQueue:adapter error:error]; return success; } @@ -1001,22 +989,24 @@ - (BOOL)getBaseDevice:(uint64_t)deviceID queue:(dispatch_queue_t)queue completio // We know getSessionForNode will return YES here, since we already checked // that we are running. - return [self getSessionForNode:deviceID - completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager, - const chip::Optional & session, NSError * _Nullable error) { - // Create an MTRBaseDevice for the node id involved, now that our - // CASE session is primed. We don't actually care about the session - // information here. - dispatch_async(queue, ^{ - MTRBaseDevice * device; - if (error == nil) { - device = [[MTRBaseDevice alloc] initWithNodeID:@(deviceID) controller:self]; - } else { - device = nil; - } - completion(device, error); - }); - }]; + [self getSessionForNode:deviceID + completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager, + const chip::Optional & session, NSError * _Nullable error) { + // Create an MTRBaseDevice for the node id involved, now that our + // CASE session is primed. We don't actually care about the session + // information here. + dispatch_async(queue, ^{ + MTRBaseDevice * device; + if (error == nil) { + device = [[MTRBaseDevice alloc] initWithNodeID:@(deviceID) controller:self]; + } else { + device = nil; + } + completion(device, error); + }); + }]; + + return YES; } - (BOOL)pairDevice:(uint64_t)deviceID diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index bed9b70162fec8..08a31c45c0ba7f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -108,31 +108,36 @@ NS_ASSUME_NONNULL_BEGIN /** * Ensure we have a CASE session to the given node ID and then call the provided * connection callback. This may be called on any queue (including the Matter - * event queue) and will always call the provided connection callback on the - * Matter queue, asynchronously. Consumers must be prepared to run on the - * Matter queue (an in particular must not use any APIs that will try to do sync - * dispatch to the Matter queue). - * - * If the controller is not running when this function is called, will return NO - * and never invoke the completion. If the controller is not running when the - * async dispatch on the Matter queue would happen, an error will be dispatched - * to the completion handler. + * event queue) and on success will always call the provided connection callback + * on the Matter queue, asynchronously. Consumers must be prepared to run on + * the Matter queue (an in particular must not use any APIs that will try to do + * sync dispatch to the Matter queue). + * + * If the controller is not running when this function is called, it will + * synchronously invoke the completion with an error, on whatever queue + * getSessionForNode was called on. + * + * If the controller is not running when the async dispatch on the Matter queue + * happens, the completion will be invoked with an error on the Matter queue. */ -- (BOOL)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion; +- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion; /** * Get a session for the commissionee device with the given device id. This may - * be called on any queue (including the Matter event queue) and will always - * call the provided connection callback on the Matter queue, asynchronously. - * Consumers must be prepared to run on the Matter queue (an in particular must - * not use any APIs that will try to do sync dispatch to the Matter queue). - * - * If the controller is not running when this function is called, will return NO - * and never invoke the completion. If the controller is not running when the - * async dispatch on the Matter queue would happen, an error will be dispatched - * to the completion handler. + * be called on any queue (including the Matter event queue) and on success will + * always call the provided connection callback on the Matter queue, + * asynchronously. Consumers must be prepared to run on the Matter queue (an in + * particular must not use any APIs that will try to do sync dispatch to the + * Matter queue). + * + * If the controller is not running when this function is called, it will + * synchronously invoke the completion with an error, on whatever queue + * getSessionForCommissioneeDevice was called on. + * + * If the controller is not running when the async dispatch on the Matter queue + * happens, the completion will be invoked with an error on the Matter queue. */ -- (BOOL)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion; +- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion; /** * Invalidate the CASE session for the given node ID. This is a temporary thing @@ -150,9 +155,22 @@ NS_ASSUME_NONNULL_BEGIN * * The DeviceCommissioner pointer passed to the callback should only be used * synchronously during the callback invocation. + * + * If the error handler is nil, failure to run the block will be silent. + */ +- (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block + errorHandler:(nullable MTRDeviceErrorHandler)errorHandler; + +/** + * Try to asynchronously dispatch the given block on the Matter queue. If the + * controller is not running either at call time or when the block would be + * about to run, the provided error handler will be called with an error. Note + * that this means the error handler might be called on an arbitrary queue, and + * might be called before this function returns or after it returns. + * + * If the error handler is nil, failure to run the block will be silent. */ -- (void)asyncDispatchToMatterQueue:(void (^)(chip::Controller::DeviceCommissioner *))block - errorHandler:(void (^)(NSError *))errorHandler; +- (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler; /** * Get an MTRBaseDevice for the given node id. This exists to allow subclasses diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm index d0d6dfc3487347..349c255e885eaa 100644 --- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm @@ -160,7 +160,7 @@ CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event) auto completionHandler = ^(NSError * _Nullable error) { [controller - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { assertChipStackLockedByCurrentThread(); if (!mInitialized || mTransferGeneration != transferGeneration) { @@ -264,7 +264,7 @@ CHIP_ERROR OnBlockQuery(TransferSession::OutputEvent & event) auto completionHandler = ^(NSData * _Nullable data, BOOL isEOF) { [controller - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { assertChipStackLockedByCurrentThread(); if (!mInitialized || mTransferGeneration != transferGeneration) { @@ -552,7 +552,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath auto completionHandler = ^(MTROTASoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data, NSError * _Nullable error) { [controller - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { assertChipStackLockedByCurrentThread(); CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "QueryImage", data, error); @@ -646,7 +646,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath auto completionHandler = ^(MTROTASoftwareUpdateProviderClusterApplyUpdateResponseParams * _Nullable data, NSError * _Nullable error) { [controller - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { assertChipStackLockedByCurrentThread(); CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "ApplyUpdateRequest", data, error); @@ -707,7 +707,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath auto completionHandler = ^(NSError * _Nullable error) { [controller - asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) { + asyncDispatchToMatterQueue:^() { assertChipStackLockedByCurrentThread(); CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "NotifyUpdateApplied", error); diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h index e0e34130aa5a8e..f31eb18a631b89 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h @@ -69,8 +69,6 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr return mCppCommissioner == nullptr ? chip::NullOptional : mCppCommissioner->GetCommissioningParameters(); } - void setChipWorkQueue(dispatch_queue_t chipWorkQueue) { mChipWorkQueue = chipWorkQueue; } - void SetOperationalCertificateIssuer( id operationalCertificateIssuer, dispatch_queue_t operationalCertificateIssuerQueue) { @@ -156,7 +154,6 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr chip::Controller::DeviceCommissioner * _Nullable mCppCommissioner = nullptr; id _Nullable mOperationalCertificateIssuer; dispatch_queue_t _Nullable mOperationalCertificateIssuerQueue; - dispatch_queue_t _Nullable mChipWorkQueue; chip::Callback::Callback * _Nullable mOnNOCCompletionCallback = nullptr; }; diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm index 40e2fd3f8028a4..59a8fe31e47933 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm @@ -22,6 +22,7 @@ #import #import "MTRCertificates.h" +#import "MTRDeviceController_Internal.h" #import "MTRLogging_Internal.h" #import "NSDataSpanConversion.h" @@ -219,54 +220,55 @@ void MTROperationalCredentialsDelegate::ExternalNOCChainGenerated( MTROperationalCertificateInfo * _Nullable info, NSError * _Nullable error) { - MTRDeviceController * __weak weakController = mWeakController; - dispatch_async(mChipWorkQueue, ^{ - MTRDeviceController * strongController = weakController; - if (strongController == nil || !strongController.isRunning) { - // No longer safe to touch "this" - return; - } - - if (mOnNOCCompletionCallback == nullptr) { - return; + // Dispatch will only happen if the controller is still running, which means we + // are safe to touch our members. + [mWeakController + asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) { + assertChipStackLockedByCurrentThread(); + + if (mOnNOCCompletionCallback == nullptr) { + return; + } + + auto * onCompletion = mOnNOCCompletionCallback; + mOnNOCCompletionCallback = nullptr; + + if (mCppCommissioner != commissioner) { + // Quite unexpected! + return; + } + + if (info == nil) { + onCompletion->mCall(onCompletion->mContext, [MTRError errorToCHIPErrorCode:error], ByteSpan(), ByteSpan(), + ByteSpan(), NullOptional, NullOptional); + return; + } + + auto commissioningParameters = commissioner->GetCommissioningParameters(); + if (!commissioningParameters.HasValue()) { + return; + } + + AesCcm128KeySpan ipk = commissioningParameters.Value().GetIpk().ValueOr(GetIPK()); + + Optional adminSubject; + if (info.adminSubject != nil) { + adminSubject.SetValue(info.adminSubject.unsignedLongLongValue); + } else { + adminSubject = commissioningParameters.Value().GetAdminSubject(); + } + + ByteSpan intermediateCertificate; + if (info.intermediateCertificate != nil) { + intermediateCertificate = AsByteSpan(info.intermediateCertificate); + } + + onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, AsByteSpan(info.operationalCertificate), + intermediateCertificate, AsByteSpan(info.rootCertificate), MakeOptional(ipk), adminSubject); } - - auto * onCompletion = mOnNOCCompletionCallback; - mOnNOCCompletionCallback = nullptr; - - if (mCppCommissioner == nullptr) { - // Quite unexpected, since we checked that our controller is running already. - return; - } - - if (info == nil) { - onCompletion->mCall(onCompletion->mContext, [MTRError errorToCHIPErrorCode:error], ByteSpan(), ByteSpan(), ByteSpan(), - NullOptional, NullOptional); - return; - } - - auto commissioningParameters = mCppCommissioner->GetCommissioningParameters(); - if (!commissioningParameters.HasValue()) { - return; - } - - AesCcm128KeySpan ipk = commissioningParameters.Value().GetIpk().ValueOr(GetIPK()); - - Optional adminSubject; - if (info.adminSubject != nil) { - adminSubject.SetValue(info.adminSubject.unsignedLongLongValue); - } else { - adminSubject = commissioningParameters.Value().GetAdminSubject(); - } - - ByteSpan intermediateCertificate; - if (info.intermediateCertificate != nil) { - intermediateCertificate = AsByteSpan(info.intermediateCertificate); - } - - onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, AsByteSpan(info.operationalCertificate), intermediateCertificate, - AsByteSpan(info.rootCertificate), MakeOptional(ipk), adminSubject); - }); + // If we can't run the block, we're torn down and should + // just do nothing. + errorHandler:nil]; } CHIP_ERROR MTROperationalCredentialsDelegate::LocalGenerateNOCChain(const chip::ByteSpan & csrElements,