[Darwin] MTRDevice_XPC delegate callbacks need to hold lock before _c…

…allDelegatesWithBlock (project-chip#35095) * [Darwin] MTRDevice_XPC delegate callbacks need to hold lock before _callDelegatesWithBlock * Add back _callDelegatesWithBlock to avoid conflict
PeterC1965 · Aug 28, 2024 · 2e3bc4e · 2e3bc4e
1 parent 09a606e
commit 2e3bc4e
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -1105,6 +1105,12 @@ - (BOOL)_callDelegatesWithBlock:(void (^)(id<MTRDeviceDelegate> delegate))block
     return (delegatesCalled > 0);
 }
 
+- (BOOL)_lockAndCallDelegatesWithBlock:(void (^)(id<MTRDeviceDelegate> delegate))block
+{
+    std::lock_guard lock(self->_lock);
+    return [self _callDelegatesWithBlock:block];
+}
+
 #ifdef DEBUG
 // Only used for unit test purposes - normal delegate should not expect or handle being called back synchronously
 - (void)_callFirstDelegateSynchronouslyWithBlock:(void (^)(id<MTRDeviceDelegate> delegate))block

diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h
@@ -144,6 +144,9 @@ MTR_DIRECT_MEMBERS
 
 - (BOOL)_callDelegatesWithBlock:(void (^)(id<MTRDeviceDelegate> delegate))block;
 
+// Called by MTRDevice_XPC to forward delegate callbacks
+- (BOOL)_lockAndCallDelegatesWithBlock:(void (^)(id<MTRDeviceDelegate> delegate))block;
+
 /**
  * Like the public invokeCommandWithEndpointID but:
  *

diff --git a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
@@ -99,23 +99,23 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
 - (oneway void)device:(NSNumber *)nodeID stateChanged:(MTRDeviceState)state
 {
     MTR_LOG("%s", __PRETTY_FUNCTION__);
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         [delegate device:self stateChanged:state];
     }];
 }
 
 - (oneway void)device:(NSNumber *)nodeID receivedAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport
 {
     MTR_LOG("%s", __PRETTY_FUNCTION__);
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         [delegate device:self receivedAttributeReport:attributeReport];
     }];
 }
 
 - (oneway void)device:(NSNumber *)nodeID receivedEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport
 {
     MTR_LOG("%s", __PRETTY_FUNCTION__);
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         [delegate device:self receivedEventReport:eventReport];
     }];
 }
@@ -124,7 +124,7 @@ - (oneway void)device:(NSNumber *)nodeID receivedEventReport:(NSArray<NSDictiona
 - (oneway void)deviceBecameActive:(NSNumber *)nodeID
 {
     MTR_LOG("%s", __PRETTY_FUNCTION__);
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         if ([delegate respondsToSelector:@selector(deviceBecameActive:)]) {
             [delegate deviceBecameActive:self];
         }
@@ -133,7 +133,7 @@ - (oneway void)deviceBecameActive:(NSNumber *)nodeID
 
 - (oneway void)deviceCachePrimed:(NSNumber *)nodeID
 {
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         if ([delegate respondsToSelector:@selector(deviceCachePrimed:)]) {
             [delegate deviceCachePrimed:self];
         }
@@ -142,7 +142,7 @@ - (oneway void)deviceCachePrimed:(NSNumber *)nodeID
 
 - (oneway void)deviceConfigurationChanged:(NSNumber *)nodeID
 {
-    [self _callDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
+    [self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
         if ([delegate respondsToSelector:@selector(deviceConfigurationChanged:)]) {
             [delegate deviceConfigurationChanged:self];
         }