Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unreachable shutdown code from MTRDeviceController. #35689

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -53,6 +53,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
Loading