[Darwin] MTRDeviceController should clear attribute cache on pairing …

…start
jtung-apple · Mar 12, 2024 · 23edbec · 23edbec
1 parent 9d2f350
commit 23edbec
Show file tree

Hide file tree

Showing 2 changed files with 173 additions and 67 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -44,6 +44,7 @@
 #import "MTRPersistentStorageDelegateBridge.h"
 #import "MTRServerEndpoint_Internal.h"
 #import "MTRSetupPayload.h"
+#import "MTRUnfairLock.h"
 #import "NSDataSpanConversion.h"
 #import "NSStringSpanConversion.h"
 #import <setup_payload/ManualSetupPayloadGenerator.h>
@@ -623,6 +624,12 @@ - (BOOL)setupCommissioningSessionWithPayload:(MTRSetupPayload *)payload
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
+    // Reset previous information about this new nodeID
+    os_unfair_lock_lock(&_deviceMapLock);
+    [self _removeDeviceWithNodeID:newNodeID device:nil];
+    os_unfair_lock_unlock(&_deviceMapLock);
+    [_controllerDataStore clearStoredAttributesForNodeID:newNodeID];
+
     return [self syncRunOnWorkQueueWithBoolReturnValue:block error:error];
 }
 
@@ -670,6 +677,12 @@ - (BOOL)setupCommissioningSessionWithDiscoveredDevice:(MTRCommissionableBrowserR
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
+    // Reset previous information about this new nodeID
+    os_unfair_lock_lock(&_deviceMapLock);
+    [self _removeDeviceWithNodeID:newNodeID device:nil];
+    os_unfair_lock_unlock(&_deviceMapLock);
+    [_controllerDataStore clearStoredAttributesForNodeID:newNodeID];
+
     return [self syncRunOnWorkQueueWithBoolReturnValue:block error:error];
 }
 
@@ -897,18 +910,30 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
     return deviceToReturn;
 }
 
-- (void)removeDevice:(MTRDevice *)device
+// Remove a device from the device map, optionally with a specific object
+- (void)_removeDeviceWithNodeID:(NSNumber *)nodeID device:(MTRDevice * _Nullable)deviceToRemove
 {
-    os_unfair_lock_lock(&_deviceMapLock);
-    auto * nodeID = device.nodeID;
-    MTRDevice * deviceToRemove = _nodeIDToDeviceMap[nodeID];
-    if (deviceToRemove == device) {
-        [deviceToRemove invalidate];
-        _nodeIDToDeviceMap[nodeID] = nil;
-    } else {
-        MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, nodeID.unsignedLongLongValue);
+    os_unfair_lock_assert_owner(&_deviceMapLock);
+
+    MTRDevice * device = _nodeIDToDeviceMap[nodeID];
+    if (!device) {
+        MTR_LOG_INFO("No device to remove with nodeID %llu", nodeID.unsignedLongLongValue);
+        return;
     }
-    os_unfair_lock_unlock(&_deviceMapLock);
+
+    if (deviceToRemove && (device != deviceToRemove)) {
+        MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", deviceToRemove, nodeID.unsignedLongLongValue);
+        return;
+    }
+
+    [deviceToRemove invalidate];
+    _nodeIDToDeviceMap[nodeID] = nil;
+}
+
+- (void)removeDevice:(MTRDevice *)device
+{
+    std::lock_guard lock(_deviceMapLock);
+    [self _removeDeviceWithNodeID:device.nodeID device:device];
 }
 
 - (void)setDeviceControllerDelegate:(id<MTRDeviceControllerDelegate>)delegate queue:(dispatch_queue_t)queue