Skip to content

Commit d63051a

Browse files
Remove unreachable shutdown code from MTRDeviceController. (#35689)
cleanupAfterStartup is no longer reachable and can be removed. At that point, the only caller of controllerShuttingDown: on the factory is MTRDeviceController_Concrete, so we can go ahead and change the argument type accordingly. At that point it becomes obvious that shutDownCppController and deinitFromFactory are only called on MTRDeviceController_Concrete instances, so those can move from MTRDeviceController_Internal to MTRDeviceController_Concrete, and the implementations in MTRDeviceController can be removed. checkForInitError is now unused on MTRDeviceController, and can be removed. And then "cleanup" is unused and can be removed.
1 parent 9214daa commit d63051a

5 files changed

+19
-131
lines changed

src/darwin/Framework/CHIP/MTRDeviceController.mm

-112
Original file line numberDiff line numberDiff line change
@@ -339,105 +339,6 @@ - (void)shutdown
339339
MTR_ABSTRACT_METHOD();
340340
}
341341

342-
// Clean up from a state where startup was called.
343-
- (void)cleanupAfterStartup
344-
{
345-
// Invalidate our MTRDevice instances before we shut down our secure
346-
// sessions and whatnot, so they don't start trying to resubscribe when we
347-
// do the secure session shutdowns. Since we don't want to hold the lock
348-
// while calling out into arbitrary invalidation code, snapshot the list of
349-
// devices before we start invalidating.
350-
MTR_LOG("%s: %@", __PRETTY_FUNCTION__, self);
351-
os_unfair_lock_lock(self.deviceMapLock);
352-
auto * devices = [self.nodeIDToDeviceMap objectEnumerator].allObjects;
353-
[_nodeIDToDeviceMap removeAllObjects];
354-
os_unfair_lock_unlock(self.deviceMapLock);
355-
356-
for (MTRDevice * device in devices) {
357-
[device invalidate];
358-
}
359-
[self stopBrowseForCommissionables];
360-
361-
[_factory controllerShuttingDown:self];
362-
}
363-
364-
// Part of cleanupAfterStartup that has to interact with the Matter work queue
365-
// in a very specific way that only MTRDeviceControllerFactory knows about.
366-
- (void)shutDownCppController
367-
{
368-
MTR_LOG("%s: %p", __PRETTY_FUNCTION__, self);
369-
assertChipStackLockedByCurrentThread();
370-
371-
// Shut down all our endpoints.
372-
for (MTRServerEndpoint * endpoint in [_serverEndpoints copy]) {
373-
[self removeServerEndpointOnMatterQueue:endpoint];
374-
}
375-
376-
if (_cppCommissioner) {
377-
auto * commissionerToShutDown = _cppCommissioner;
378-
// Flag ourselves as not running before we start shutting down
379-
// _cppCommissioner, so we're not in a state where we claim to be
380-
// running but are actually partially shut down.
381-
_cppCommissioner = nullptr;
382-
commissionerToShutDown->Shutdown();
383-
// Don't clear out our fabric index association until controller
384-
// shutdown completes, in case it wants to write to storage as it
385-
// shuts down.
386-
_storedFabricIndex = chip::kUndefinedFabricIndex;
387-
_storedCompressedFabricID = std::nullopt;
388-
self.nodeID = nil;
389-
self.fabricID = nil;
390-
self.rootPublicKey = nil;
391-
392-
delete commissionerToShutDown;
393-
if (_operationalCredentialsDelegate != nil) {
394-
_operationalCredentialsDelegate->SetDeviceCommissioner(nullptr);
395-
}
396-
}
397-
}
398-
399-
- (void)deinitFromFactory
400-
{
401-
[self cleanup];
402-
}
403-
404-
// Clean up any members we might have allocated.
405-
- (void)cleanup
406-
{
407-
VerifyOrDie(_cppCommissioner == nullptr);
408-
409-
if (_defaultDACVerifier) {
410-
delete _defaultDACVerifier;
411-
_defaultDACVerifier = nullptr;
412-
}
413-
414-
if (_attestationTrustStoreBridge) {
415-
delete _attestationTrustStoreBridge;
416-
_attestationTrustStoreBridge = nullptr;
417-
}
418-
419-
[self clearDeviceAttestationDelegateBridge];
420-
421-
if (_operationalCredentialsDelegate) {
422-
delete _operationalCredentialsDelegate;
423-
_operationalCredentialsDelegate = nullptr;
424-
}
425-
426-
if (_partialDACVerifier) {
427-
delete _partialDACVerifier;
428-
_partialDACVerifier = nullptr;
429-
}
430-
431-
if (_deviceControllerDelegateBridge) {
432-
delete _deviceControllerDelegateBridge;
433-
_deviceControllerDelegateBridge = nullptr;
434-
@synchronized(self) {
435-
_strongDelegateForSetDelegateAPI = nil;
436-
[_delegates removeAllObjects];
437-
}
438-
}
439-
}
440-
441342
- (NSNumber *)controllerNodeID
442343
{
443344
auto block = ^NSNumber * { return @(self->_cppCommissioner->GetNodeId()); };
@@ -968,19 +869,6 @@ - (void)removeServerEndpointOnMatterQueue:(MTRServerEndpoint *)endpoint
968869
[_factory removeServerEndpoint:endpoint];
969870
}
970871

971-
- (BOOL)checkForInitError:(BOOL)condition logMsg:(NSString *)logMsg
972-
{
973-
if (condition) {
974-
return NO;
975-
}
976-
977-
MTR_LOG_ERROR("%@ Error: %@", self, logMsg);
978-
979-
[self cleanup];
980-
981-
return YES;
982-
}
983-
984872
- (void)clearDeviceAttestationDelegateBridge
985873
{
986874
if (_deviceAttestationDelegateBridge) {

src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ - (void)resetOperationalAdvertising
906906
app::DnssdServer::Instance().StartServer();
907907
}
908908

909-
- (void)controllerShuttingDown:(MTRDeviceController *)controller
909+
- (void)controllerShuttingDown:(MTRDeviceController_Concrete *)controller
910910
{
911911
[self _assertCurrentQueueIsNotMatterQueue];
912912

src/darwin/Framework/CHIP/MTRDeviceControllerFactory_Internal.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ NS_ASSUME_NONNULL_BEGIN
5151
MTR_DIRECT_MEMBERS
5252
@interface MTRDeviceControllerFactory ()
5353

54-
- (void)controllerShuttingDown:(MTRDeviceController *)controller;
54+
- (void)controllerShuttingDown:(MTRDeviceController_Concrete *)controller;
5555

5656
/**
5757
* Get the list of running controllers. This will include controllers that are

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h

+17
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,23 @@ NS_ASSUME_NONNULL_BEGIN
5353
*/
5454
- (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams;
5555

56+
/**
57+
* Shut down the underlying C++ controller. Must be called on the Matter work
58+
* queue or after the Matter work queue has been shut down.
59+
*
60+
* Only MTRDeviceControllerFactory should be calling this.
61+
*/
62+
- (void)shutDownCppController;
63+
64+
/**
65+
* Notification that the MTRDeviceControllerFactory has finished shutting down
66+
* this controller and will not be touching it anymore. This is guaranteed to
67+
* be called after initWithFactory succeeds.
68+
*
69+
* Only MTRDeviceControllerFactory should be calling this.
70+
*/
71+
- (void)deinitFromFactory;
72+
5673
/**
5774
* Takes an assertion to keep the controller running. If `-[MTRDeviceController shutdown]` is called while an assertion
5875
* is held, the shutdown will be honored only after all assertions are released. Invoking this method multiple times increases

src/darwin/Framework/CHIP/MTRDeviceController_Internal.h

-17
Original file line numberDiff line numberDiff line change
@@ -136,23 +136,6 @@ NS_ASSUME_NONNULL_BEGIN
136136
fabricIndex:(chip::FabricIndex)fabricIndex
137137
isRunning:(BOOL *)isRunning;
138138

139-
/**
140-
* Shut down the underlying C++ controller. Must be called on the Matter work
141-
* queue or after the Matter work queue has been shut down.
142-
*
143-
* Only MTRDeviceControllerFactory should be calling this.
144-
*/
145-
- (void)shutDownCppController;
146-
147-
/**
148-
* Notification that the MTRDeviceControllerFactory has finished shutting down
149-
* this controller and will not be touching it anymore. This is guaranteed to
150-
* be called after initWithFactory succeeds.
151-
*
152-
* Only MTRDeviceControllerFactory should be calling this.
153-
*/
154-
- (void)deinitFromFactory;
155-
156139
/**
157140
* Ensure we have a CASE session to the given node ID and then call the provided
158141
* connection callback. This may be called on any queue (including the Matter

0 commit comments

Comments
 (0)