Skip to content

Commit

Permalink
Remove unreachable shutdown code from MTRDeviceController.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple committed Sep 20, 2024
1 parent 56c0bd8 commit edb4014
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 131 deletions.
112 changes: 0 additions & 112 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -339,105 +339,6 @@ - (void)shutdown
MTR_ABSTRACT_METHOD();
}

// Clean up from a state where startup was called.
- (void)cleanupAfterStartup
{
// Invalidate our MTRDevice instances before we shut down our secure
// sessions and whatnot, so they don't start trying to resubscribe when we
// do the secure session shutdowns. Since we don't want to hold the lock
// while calling out into arbitrary invalidation code, snapshot the list of
// devices before we start invalidating.
MTR_LOG("%s: %@", __PRETTY_FUNCTION__, self);
os_unfair_lock_lock(self.deviceMapLock);
auto * devices = [self.nodeIDToDeviceMap objectEnumerator].allObjects;
[_nodeIDToDeviceMap removeAllObjects];
os_unfair_lock_unlock(self.deviceMapLock);

for (MTRDevice * device in devices) {
[device invalidate];
}
[self stopBrowseForCommissionables];

[_factory controllerShuttingDown:self];
}

// Part of cleanupAfterStartup that has to interact with the Matter work queue
// in a very specific way that only MTRDeviceControllerFactory knows about.
- (void)shutDownCppController
{
MTR_LOG("%s: %p", __PRETTY_FUNCTION__, self);
assertChipStackLockedByCurrentThread();

// Shut down all our endpoints.
for (MTRServerEndpoint * endpoint in [_serverEndpoints copy]) {
[self removeServerEndpointOnMatterQueue:endpoint];
}

if (_cppCommissioner) {
auto * commissionerToShutDown = _cppCommissioner;
// Flag ourselves as not running before we start shutting down
// _cppCommissioner, so we're not in a state where we claim to be
// running but are actually partially shut down.
_cppCommissioner = nullptr;
commissionerToShutDown->Shutdown();
// Don't clear out our fabric index association until controller
// shutdown completes, in case it wants to write to storage as it
// shuts down.
_storedFabricIndex = chip::kUndefinedFabricIndex;
_storedCompressedFabricID = std::nullopt;
self.nodeID = nil;
self.fabricID = nil;
self.rootPublicKey = nil;

delete commissionerToShutDown;
if (_operationalCredentialsDelegate != nil) {
_operationalCredentialsDelegate->SetDeviceCommissioner(nullptr);
}
}
}

- (void)deinitFromFactory
{
[self cleanup];
}

// Clean up any members we might have allocated.
- (void)cleanup
{
VerifyOrDie(_cppCommissioner == nullptr);

if (_defaultDACVerifier) {
delete _defaultDACVerifier;
_defaultDACVerifier = nullptr;
}

if (_attestationTrustStoreBridge) {
delete _attestationTrustStoreBridge;
_attestationTrustStoreBridge = nullptr;
}

[self clearDeviceAttestationDelegateBridge];

if (_operationalCredentialsDelegate) {
delete _operationalCredentialsDelegate;
_operationalCredentialsDelegate = nullptr;
}

if (_partialDACVerifier) {
delete _partialDACVerifier;
_partialDACVerifier = nullptr;
}

if (_deviceControllerDelegateBridge) {
delete _deviceControllerDelegateBridge;
_deviceControllerDelegateBridge = nullptr;
@synchronized(self) {
_strongDelegateForSetDelegateAPI = nil;
[_delegates removeAllObjects];
}
}
}

- (NSNumber *)controllerNodeID
{
auto block = ^NSNumber * { return @(self->_cppCommissioner->GetNodeId()); };
Expand Down Expand Up @@ -968,19 +869,6 @@ - (void)removeServerEndpointOnMatterQueue:(MTRServerEndpoint *)endpoint
[_factory removeServerEndpoint:endpoint];
}

- (BOOL)checkForInitError:(BOOL)condition logMsg:(NSString *)logMsg
{
if (condition) {
return NO;
}

MTR_LOG_ERROR("%@ Error: %@", self, logMsg);

[self cleanup];

return YES;
}

- (void)clearDeviceAttestationDelegateBridge
{
if (_deviceAttestationDelegateBridge) {
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ - (void)resetOperationalAdvertising
app::DnssdServer::Instance().StartServer();
}

- (void)controllerShuttingDown:(MTRDeviceController *)controller
- (void)controllerShuttingDown:(MTRDeviceController_Concrete *)controller
{
[self _assertCurrentQueueIsNotMatterQueue];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ NS_ASSUME_NONNULL_BEGIN
MTR_DIRECT_MEMBERS
@interface MTRDeviceControllerFactory ()

- (void)controllerShuttingDown:(MTRDeviceController *)controller;
- (void)controllerShuttingDown:(MTRDeviceController_Concrete *)controller;

/**
* Get the list of running controllers. This will include controllers that are
Expand Down
17 changes: 17 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams;

/**
* Shut down the underlying C++ controller. Must be called on the Matter work
* queue or after the Matter work queue has been shut down.
*
* Only MTRDeviceControllerFactory should be calling this.
*/
- (void)shutDownCppController;

/**
* Notification that the MTRDeviceControllerFactory has finished shutting down
* this controller and will not be touching it anymore. This is guaranteed to
* be called after initWithFactory succeeds.
*
* Only MTRDeviceControllerFactory should be calling this.
*/
- (void)deinitFromFactory;

/**
* 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
Expand Down
17 changes: 0 additions & 17 deletions src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,6 @@ NS_ASSUME_NONNULL_BEGIN
fabricIndex:(chip::FabricIndex)fabricIndex
isRunning:(BOOL *)isRunning;

/**
* Shut down the underlying C++ controller. Must be called on the Matter work
* queue or after the Matter work queue has been shut down.
*
* Only MTRDeviceControllerFactory should be calling this.
*/
- (void)shutDownCppController;

/**
* Notification that the MTRDeviceControllerFactory has finished shutting down
* this controller and will not be touching it anymore. This is guaranteed to
* be called after initWithFactory succeeds.
*
* Only MTRDeviceControllerFactory should be calling this.
*/
- (void)deinitFromFactory;

/**
* 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
Expand Down

0 comments on commit edb4014

Please sign in to comment.