Remove shutdown implementation from base MTRDeviceController. (#35676)
shutdown is overriden by both MTRDeviceController_Concrete and
MTRDeviceController_XPC, so the base class implementation is not reachable. And
it's the only caller of finalShutdown, so that can also be removed.
Also, addRunAssertion/removeRunAssertion, are only used on concrete
controllers and can be removed from the base class and from
MTRDeviceController_Internal.
With those removed, matchesPendingShutdownControllerWithOperationalCertificate
would always return false, so that can be changed accordingly, and then
clearPendingShutdown becomes unreachable.
At this point _keepRunningAssertionCounter and _shutdownPending are never read
and can be removed. And _assertionLock is never acquired, so it can also be
removed.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
index e39a4d1..4c3afe8 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -26,6 +26,7 @@
#import "MTRCommissionableBrowserResult_Internal.h"
#import "MTRCommissioningParameters.h"
#import "MTRConversion.h"
+#import "MTRDefines_Internal.h"
#import "MTRDeviceControllerDelegateBridge.h"
#import "MTRDeviceControllerFactory_Internal.h"
#import "MTRDeviceControllerLocalTestStorage.h"
@@ -160,11 +161,6 @@
// specific queue, so can't race against each other.
std::atomic<bool> _suspended;
- // Counters to track assertion status and access controlled by the _assertionLock
- NSUInteger _keepRunningAssertionCounter;
- BOOL _shutdownPending;
- os_unfair_lock _assertionLock;
-
NSMutableArray<MTRDeviceControllerDelegateInfo *> * _delegates;
id<MTRDeviceControllerDelegate> _strongDelegateForSetDelegateAPI;
}
@@ -183,11 +179,6 @@
}
_underlyingDeviceMapLock = OS_UNFAIR_LOCK_INIT;
- // Setup assertion variables
- _keepRunningAssertionCounter = 0;
- _shutdownPending = NO;
- _assertionLock = OS_UNFAIR_LOCK_INIT;
-
_suspended = startSuspended;
_nodeIDToDeviceMap = [NSMapTable strongToWeakObjectsMapTable];
@@ -231,11 +222,6 @@
// before we start doing anything else with the controller.
_uniqueIdentifier = uniqueIdentifier;
- // Setup assertion variables
- _keepRunningAssertionCounter = 0;
- _shutdownPending = NO;
- _assertionLock = OS_UNFAIR_LOCK_INIT;
-
_suspended = startSuspended;
if (storageDelegate != nil) {
@@ -478,75 +464,21 @@
- (BOOL)matchesPendingShutdownControllerWithOperationalCertificate:(nullable MTRCertificateDERBytes)operationalCertificate andRootCertificate:(nullable MTRCertificateDERBytes)rootCertificate
{
- if (!operationalCertificate || !rootCertificate) {
- return FALSE;
- }
- NSNumber * nodeID = [MTRDeviceControllerParameters nodeIDFromNOC:operationalCertificate];
- NSNumber * fabricID = [MTRDeviceControllerParameters fabricIDFromNOC:operationalCertificate];
- NSData * publicKey = [MTRDeviceControllerParameters publicKeyFromCertificate:rootCertificate];
-
- std::lock_guard lock(_assertionLock);
-
- // If any of the local above are nil, the return will be false since MTREqualObjects handles them correctly
- return _keepRunningAssertionCounter > 0 && _shutdownPending && MTREqualObjects(nodeID, self.nodeID) && MTREqualObjects(fabricID, self.fabricID) && MTREqualObjects(publicKey, self.rootPublicKey);
-}
-
-- (void)addRunAssertion
-{
- std::lock_guard lock(_assertionLock);
-
- // Only take an assertion if running
- if ([self isRunning]) {
- ++_keepRunningAssertionCounter;
- MTR_LOG("%@ Adding keep running assertion, total %lu", self, static_cast<unsigned long>(_keepRunningAssertionCounter));
- }
-}
-
-- (void)removeRunAssertion;
-{
- std::lock_guard lock(_assertionLock);
-
- if (_keepRunningAssertionCounter > 0) {
- --_keepRunningAssertionCounter;
- MTR_LOG("%@ Removing keep running assertion, total %lu", self, static_cast<unsigned long>(_keepRunningAssertionCounter));
-
- if ([self isRunning] && _keepRunningAssertionCounter == 0 && _shutdownPending) {
- MTR_LOG("%@ All assertions removed and shutdown is pending, shutting down", self);
- [self finalShutdown];
- }
- }
+ // TODO: Once the factory knows it's dealing with MTRDeviceController_Concrete, this can be removed, and its
+ // declaration moved to MTRDeviceController_Concrete.
+ return NO;
}
- (void)clearPendingShutdown
{
- std::lock_guard lock(_assertionLock);
- _shutdownPending = NO;
+ // TODO: Once the factory knows it's dealing with MTRDeviceController_Concrete, this can be removed, and its
+ // declaration moved to MTRDeviceController_Concrete.
+ MTR_ABSTRACT_METHOD();
}
- (void)shutdown
{
- std::lock_guard lock(_assertionLock);
-
- if (_keepRunningAssertionCounter > 0) {
- MTR_LOG("%@ Pending shutdown since %lu assertions are present", self, static_cast<unsigned long>(_keepRunningAssertionCounter));
- _shutdownPending = YES;
- return;
- }
- [self finalShutdown];
-}
-
-- (void)finalShutdown
-{
- os_unfair_lock_assert_owner(&_assertionLock);
-
- MTR_LOG("%@ shutdown called", self);
- if (_cppCommissioner == nullptr) {
- // Already shut down.
- return;
- }
-
- MTR_LOG("Shutting down %@: %@", NSStringFromClass(self.class), self);
- [self cleanupAfterStartup];
+ MTR_ABSTRACT_METHOD();
}
// Clean up from a state where startup was called.
@@ -604,7 +536,6 @@
_operationalCredentialsDelegate->SetDeviceCommissioner(nullptr);
}
}
- _shutdownPending = NO;
}
- (void)deinitFromFactory
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
index ee0f330..c44b6e3 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
@@ -1144,6 +1144,9 @@
{
std::lock_guard lock(_controllersLock);
for (MTRDeviceController * controller in _controllers) {
+ // TODO: Once we know our controllers are MTRDeviceController_Concrete, move
+ // matchesPendingShutdownControllerWithOperationalCertificate and clearPendingShutdown to that
+ // interface and remove them from base MTRDeviceController_Internal.
if ([controller matchesPendingShutdownControllerWithOperationalCertificate:operationalCertificate andRootCertificate:rootCertificate]) {
MTR_LOG("%@ Found existing controller %@ that is pending shutdown and matching parameters, re-using it", self, controller);
[controller clearPendingShutdown];
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
index b491e3f..59dd42c 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
@@ -23,6 +23,21 @@
NS_ASSUME_NONNULL_BEGIN
@interface MTRDeviceController_Concrete : MTRDeviceController
+
+/**
+ * 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
+ * the number of assertions and needs to be matched with equal amount of '-[MTRDeviceController removeRunAssertion]` to release
+ * the assertion.
+ */
+- (void)addRunAssertion;
+
+/**
+ * Removes an assertion to allow the controller to shutdown once all assertions have been released.
+ * Invoking this method once all assertions have been released in a noop.
+ */
+- (void)removeRunAssertion;
+
@end
NS_ASSUME_NONNULL_END
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
index 9932982..8eb21ae 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
@@ -309,20 +309,6 @@
- (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
/**
- * 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
- * the number of assertions and needs to be matched with equal amount of '-[MTRDeviceController removeRunAssertion]` to release
- * the assertion.
- */
-- (void)addRunAssertion;
-
-/**
- * Removes an assertion to allow the controller to shutdown once all assertions have been released.
- * Invoking this method once all assertions have been released in a noop.
- */
-- (void)removeRunAssertion;
-
-/**
* This method returns TRUE if this controller matches the fabric reference and node ID as listed in the parameters.
*/
- (BOOL)matchesPendingShutdownControllerWithOperationalCertificate:(nullable MTRCertificateDERBytes)operationalCertificate andRootCertificate:(nullable MTRCertificateDERBytes)rootCertificate;
