diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index ccefbb18497414..866f52481c42e5 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -1795,11 +1795,7 @@ static CHIP_ERROR OpenCommissioningWindow(Controller::DeviceController * control
CHIP_ERROR OpenCommissioningWindowHelper::OpenCommissioningWindow(Controller::DeviceController * controller, NodeId nodeID,
System::Clock::Seconds16 timeout, uint16_t discriminator, const Optional<uint32_t> & setupPIN, ResultCallback callback)
{
- auto * self = new (std::nothrow) OpenCommissioningWindowHelper(controller, callback);
- if (self == nullptr) {
- return CHIP_ERROR_NO_MEMORY;
- }
-
+ auto * self = new OpenCommissioningWindowHelper(controller, callback);
SetupPayload unused;
CHIP_ERROR err = self->mOpener.OpenCommissioningWindow(nodeID, timeout, Crypto::kSpake2p_Min_PBKDF_Iterations, discriminator,
setupPIN, NullOptional, &self->mOnOpenCommissioningWindowCallback, unused);
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
index 87490eecf6e7a6..e9610de2fea0a7 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -413,10 +413,6 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
}
_cppCommissioner = new chip::Controller::DeviceCommissioner();
- if (_cppCommissioner == nullptr) {
- [self checkForStartError:CHIP_ERROR_NO_MEMORY logMsg:kErrorCommissionerInit];
- return;
- }
// nocBuffer might not be used, but if it is it needs to live
// long enough (until after we are done using
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
index 4b5ccc7d9477d4..25939d6ae5c64e 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
@@ -67,12 +67,6 @@
using namespace chip;
using namespace chip::Controller;
-static NSString * const kErrorPersistentStorageInit = @"Init failure while creating a persistent storage delegate";
-static NSString * const kErrorSessionResumptionStorageInit = @"Init failure while creating a session resumption storage delegate";
-static NSString * const kErrorControllerFactoryInit = @"Init failure while initializing controller factory";
-static NSString * const kErrorKeystoreInit = @"Init failure while initializing persistent storage keystore";
-static NSString * const kErrorCertStoreInit = @"Init failure while initializing persistent storage operational certificate store";
-
static bool sExitHandlerRegistered = false;
static void ShutdownOnExit()
{
@@ -228,21 +222,6 @@ - (void)_assertCurrentQueueIsNotMatterQueue
VerifyOrDie(!DeviceLayer::PlatformMgrImpl().IsWorkQueueCurrentQueue());
}
-- (BOOL)checkIsRunning:(NSError * __autoreleasing *)error
-{
- [self _assertCurrentQueueIsNotMatterQueue];
-
- if ([self isRunning]) {
- return YES;
- }
-
- if (error != nil) {
- *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE];
- }
-
- return NO;
-}
-
- (void)cleanupStartupObjects
{
assertChipStackLockedByCurrentThread();
@@ -293,7 +272,7 @@ - (CHIP_ERROR)_initFabricTable:(FabricTable &)fabricTable
{
[self _assertCurrentQueueIsNotMatterQueue];
- if (!self.isRunning) {
+ if (!_running) { // Note: reading _running from outside of the Matter work queue
return nil;
}
@@ -339,40 +318,25 @@ - (BOOL)_startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParam
{
[self _assertCurrentQueueIsNotMatterQueue];
- if ([self isRunning]) {
- MTR_LOG_DEBUG("Ignoring duplicate call to startup, Matter controller factory already started...");
- return YES;
- }
-
- __block CHIP_ERROR errorCode = CHIP_NO_ERROR;
+ __block CHIP_ERROR err = CHIP_ERROR_INTERNAL;
dispatch_sync(_chipWorkQueue, ^{
- if ([self isRunning]) {
- return;
+ if (_running) {
+ // TODO: When treating a duplicate call as success we should validate parameters match
+ MTR_LOG_DEBUG("Ignoring duplicate call to startup, Matter controller factory already started...");
+ ExitNow(err = CHIP_NO_ERROR);
}
StartupMetricsCollection();
InitializeServerAccessControl();
if (startupParams.hasStorage) {
- _persistentStorageDelegate = new (std::nothrow) MTRPersistentStorageDelegateBridge(startupParams.storage);
+ _persistentStorageDelegate = new MTRPersistentStorageDelegateBridge(startupParams.storage);
_sessionResumptionStorage = nullptr;
_usingPerControllerStorage = NO;
} else {
- _persistentStorageDelegate = new (std::nothrow) MTRDemuxingStorage(self);
- _sessionResumptionStorage = new (std::nothrow) MTRSessionResumptionStorageBridge(self);
+ _persistentStorageDelegate = new MTRDemuxingStorage(self);
+ _sessionResumptionStorage = new MTRSessionResumptionStorageBridge(self);
_usingPerControllerStorage = YES;
-
- if (_sessionResumptionStorage == nil) {
- MTR_LOG_ERROR("Error: %@", kErrorSessionResumptionStorageInit);
- errorCode = CHIP_ERROR_NO_MEMORY;
- return;
- }
- }
-
- if (_persistentStorageDelegate == nil) {
- MTR_LOG_ERROR("Error: %@", kErrorPersistentStorageInit);
- errorCode = CHIP_ERROR_NO_MEMORY;
- return;
}
_otaProviderDelegate = startupParams.otaProviderDelegate;
@@ -383,63 +347,42 @@ - (BOOL)_startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParam
// TODO: Allow passing a different keystore implementation via startupParams.
_keystore = new PersistentStorageOperationalKeystore();
- if (_keystore == nullptr) {
- MTR_LOG_ERROR("Error: %@", kErrorKeystoreInit);
- errorCode = CHIP_ERROR_NO_MEMORY;
- return;
- }
-
- errorCode = _keystore->Init(_persistentStorageDelegate);
- if (errorCode != CHIP_NO_ERROR) {
- MTR_LOG_ERROR("Error: %@", kErrorKeystoreInit);
- return;
- }
+ SuccessOrExit(err = _keystore->Init(_persistentStorageDelegate));
// TODO Allow passing a different opcert store implementation via startupParams.
_opCertStore = new Credentials::PersistentStorageOpCertStore();
- if (_opCertStore == nullptr) {
- MTR_LOG_ERROR("Error: %@", kErrorCertStoreInit);
- errorCode = CHIP_ERROR_NO_MEMORY;
- return;
- }
-
- errorCode = _opCertStore->Init(_persistentStorageDelegate);
- if (errorCode != CHIP_NO_ERROR) {
- MTR_LOG_ERROR("Error: %@", kErrorCertStoreInit);
- return;
- }
+ SuccessOrExit(err = _opCertStore->Init(_persistentStorageDelegate));
_productAttestationAuthorityCertificates = [startupParams.productAttestationAuthorityCertificates copy];
_certificationDeclarationCertificates = [startupParams.certificationDeclarationCertificates copy];
- chip::Controller::FactoryInitParams params;
- if (startupParams.port != nil) {
- params.listenPort = [startupParams.port unsignedShortValue];
- }
- params.enableServerInteractions = startupParams.shouldStartServer;
-
- params.groupDataProvider = &_groupDataProvider;
- params.sessionKeystore = &_sessionKeystore;
- params.fabricIndependentStorage = _persistentStorageDelegate;
- params.operationalKeystore = _keystore;
- params.opCertStore = _opCertStore;
- params.certificateValidityPolicy = &_certificateValidityPolicy;
- params.sessionResumptionStorage = _sessionResumptionStorage;
- errorCode = _controllerFactory->Init(params);
- if (errorCode != CHIP_NO_ERROR) {
- MTR_LOG_ERROR("Error: %@", kErrorControllerFactoryInit);
- return;
+ {
+ chip::Controller::FactoryInitParams params;
+ if (startupParams.port != nil) {
+ params.listenPort = [startupParams.port unsignedShortValue];
+ }
+ params.enableServerInteractions = startupParams.shouldStartServer;
+
+ params.groupDataProvider = &_groupDataProvider;
+ params.sessionKeystore = &_sessionKeystore;
+ params.fabricIndependentStorage = _persistentStorageDelegate;
+ params.operationalKeystore = _keystore;
+ params.opCertStore = _opCertStore;
+ params.certificateValidityPolicy = &_certificateValidityPolicy;
+ params.sessionResumptionStorage = _sessionResumptionStorage;
+ SuccessOrExit(err = _controllerFactory->Init(params));
}
// This needs to happen after DeviceControllerFactory::Init,
// because that creates (lazily, by calling functions with
// static variables in them) some static-lifetime objects.
if (!sExitHandlerRegistered) {
- int ret = atexit(ShutdownOnExit);
- if (ret != 0) {
- MTR_LOG_ERROR("Error registering exit handler: %d", ret);
- return;
+ if (atexit(ShutdownOnExit) != 0) {
+ char error[128];
+ strerror_r(errno, error, sizeof(error));
+ MTR_LOG_ERROR("Warning: Failed to register atexit handler: %s", error);
}
+ sExitHandlerRegistered = true;
}
HeapObjectPoolExitHandling::IgnoreLeaksOnExit();
@@ -452,20 +395,24 @@ - (BOOL)_startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParam
_controllerFactory->RetainSystemState();
_controllerFactory->ReleaseSystemState();
- self->_advertiseOperational = startupParams.shouldStartServer;
- self->_running = YES;
- });
+ _advertiseOperational = startupParams.shouldStartServer;
+ _running = YES;
+ err = CHIP_NO_ERROR;
- if (![self isRunning]) {
- dispatch_sync(_chipWorkQueue, ^{
+ exit:
+ if (err != CHIP_NO_ERROR) {
+ // Note: Since we have failed no later than _controllerFactory->Init(),
+ // there is no need to call _controllerFactory->Shutdown() here.
[self cleanupStartupObjects];
- });
+ }
+ });
+ if (err != CHIP_NO_ERROR) {
+ MTR_LOG_ERROR("Failed to start Matter controller factory: %" CHIP_ERROR_FORMAT, err.Format());
if (error != nil) {
- *error = [MTRError errorForCHIPErrorCode:errorCode];
+ *error = [MTRError errorForCHIPErrorCode:err];
}
return NO;
}
-
return YES;
}
@@ -473,26 +420,19 @@ - (void)stopControllerFactory
{
[self _assertCurrentQueueIsNotMatterQueue];
- if (![self isRunning]) {
- return;
- }
-
while ([_controllers count] != 0) {
[_controllers[0] shutdown];
}
dispatch_sync(_chipWorkQueue, ^{
+ VerifyOrReturn(_running);
+
MTR_LOG_INFO("Shutting down the Matter controller factory");
_controllerFactory->Shutdown();
-
[self cleanupStartupObjects];
+ _running = NO;
+ _advertiseOperational = NO;
});
-
- // NOTE: we do not call cleanupInitObjects because we can be restarted, and
- // that does not re-create the objects that we create inside init.
- // Maybe we should be creating them in startup?
-
- _running = NO;
}
/**
@@ -540,7 +480,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
return nil;
}
- if (![self isRunning]) {
+ if (!_running) { // Note: reading _running from outside of the Matter work queue
if (storageDelegate != nil) {
MTR_LOG_DEFAULT("Auto-starting Matter controller factory in per-controller storage mode");
auto * params = [[MTRDeviceControllerFactoryParams alloc] initWithoutStorage];
@@ -744,13 +684,8 @@ - (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceCo
keystore:self->_keystore
advertiseOperational:self->_advertiseOperational
params:startupParams];
- if (params == nil) {
- fabricError = CHIP_ERROR_NO_MEMORY;
- } else {
- params.productAttestationAuthorityCertificates = self->_productAttestationAuthorityCertificates;
- params.certificationDeclarationCertificates = self->_certificationDeclarationCertificates;
- }
-
+ params.productAttestationAuthorityCertificates = self->_productAttestationAuthorityCertificates;
+ params.certificationDeclarationCertificates = self->_certificationDeclarationCertificates;
return params;
}
error:error];
@@ -800,12 +735,8 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl
keystore:self->_keystore
advertiseOperational:self->_advertiseOperational
params:startupParams];
- if (params == nil) {
- fabricError = CHIP_ERROR_NO_MEMORY;
- } else {
- params.productAttestationAuthorityCertificates = self->_productAttestationAuthorityCertificates;
- params.certificationDeclarationCertificates = self->_certificationDeclarationCertificates;
- }
+ params.productAttestationAuthorityCertificates = self->_productAttestationAuthorityCertificates;
+ params.certificationDeclarationCertificates = self->_certificationDeclarationCertificates;
return params;
}
error:error];
@@ -907,12 +838,6 @@ - (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceControll
- (void)resetOperationalAdvertising
{
- if (![self checkIsRunning:nil]) {
- // No need to reset anything; we are not running, so not
- // advertising.
- return;
- }
-
if (!_advertiseOperational) {
// No need to reset anything; we are not advertising the things that
// would need to get reset.
diff --git a/src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm b/src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm
index 2f909fdc4b2e37..c1716b513b38a6 100644
--- a/src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm
+++ b/src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm
@@ -50,19 +50,11 @@ - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
chip::SetupPayload cPlusPluspayload;
MTRSetupPayload * payload;
- if (_chipManualSetupPayloadParser) {
- CHIP_ERROR chipError = _chipManualSetupPayloadParser->populatePayload(cPlusPluspayload);
-
- if (chipError == CHIP_NO_ERROR) {
- payload = [[MTRSetupPayload alloc] initWithSetupPayload:cPlusPluspayload];
- } else if (error) {
- *error = [MTRError errorForCHIPErrorCode:chipError];
- }
- } else {
- // Memory init has failed
- if (error) {
- *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY];
- }
+ CHIP_ERROR chipError = _chipManualSetupPayloadParser->populatePayload(cPlusPluspayload);
+ if (chipError == CHIP_NO_ERROR) {
+ payload = [[MTRSetupPayload alloc] initWithSetupPayload:cPlusPluspayload];
+ } else if (error) {
+ *error = [MTRError errorForCHIPErrorCode:chipError];
}
return payload;
diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm
index 888e3ddba38bfc..fa280c81e63bab 100644
--- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm
+++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm
@@ -64,17 +64,8 @@
// Make copies of the certificates, just in case the API consumer
// has them as MutableData.
- mRootCert = [NSData dataWithData:rootCert];
- if (mRootCert == nil) {
- return CHIP_ERROR_NO_MEMORY;
- }
-
- if (icaCert != nil) {
- mIntermediateCert = [NSData dataWithData:icaCert];
- if (mIntermediateCert == nil) {
- return CHIP_ERROR_NO_MEMORY;
- }
- }
+ mRootCert = [rootCert copy];
+ mIntermediateCert = [icaCert copy];
return CHIP_NO_ERROR;
}
diff --git a/src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm b/src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm
index 53b8e733367a4f..acfa613bc6626f 100644
--- a/src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm
+++ b/src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm
@@ -50,19 +50,11 @@ - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
chip::SetupPayload cPlusPluspayload;
MTRSetupPayload * payload;
- if (_chipQRCodeSetupPayloadParser) {
- CHIP_ERROR chipError = _chipQRCodeSetupPayloadParser->populatePayload(cPlusPluspayload);
-
- if (chipError == CHIP_NO_ERROR) {
- payload = [[MTRSetupPayload alloc] initWithSetupPayload:cPlusPluspayload];
- } else if (error) {
- *error = [MTRError errorForCHIPErrorCode:chipError];
- }
- } else {
- // Memory init has failed
- if (error) {
- *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY];
- }
+ CHIP_ERROR chipError = _chipQRCodeSetupPayloadParser->populatePayload(cPlusPluspayload);
+ if (chipError == CHIP_NO_ERROR) {
+ payload = [[MTRSetupPayload alloc] initWithSetupPayload:cPlusPluspayload];
+ } else if (error) {
+ *error = [MTRError errorForCHIPErrorCode:chipError];
}
return payload;
diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m
index 63a5baf32b64d5..975625ea55df4b 100644
--- a/src/darwin/Framework/CHIPTests/MTRControllerTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRControllerTests.m
@@ -62,6 +62,10 @@ - (void)testFactoryLifecycle
XCTAssertTrue([factory startControllerFactory:factoryParams error:nil]);
XCTAssertTrue([factory isRunning]);
+ // Starting again with identical params is a no-op
+ __auto_type * factoryParams2 = [[MTRDeviceControllerFactoryParams alloc] initWithStorage:storage];
+ XCTAssertTrue([factory startControllerFactory:factoryParams2 error:nil]);
+
[factory stopControllerFactory];
XCTAssertFalse([factory isRunning]);