From 2e3bc4ed3592898d33353bc99ee4e8f0da1d50d3 Mon Sep 17 00:00:00 2001
From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com>
Date: Tue, 20 Aug 2024 12:53:27 -0700
Subject: [PATCH] [Darwin] MTRDevice_XPC delegate callbacks need to hold lock
 before _callDelegatesWithBlock (#35095)

* [Darwin] MTRDevice_XPC delegate callbacks need to hold lock before _callDelegatesWithBlock

* Add back _callDelegatesWithBlock to avoid conflict
---
 src/darwin/Framework/CHIP/MTRDevice.mm         |  6 ++++++
 src/darwin/Framework/CHIP/MTRDevice_Internal.h |  3 +++
 src/darwin/Framework/CHIP/MTRDevice_XPC.mm     | 12 ++++++------
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index b2fc853fb9b955..ff4a76147d855f 100644
--- 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
index b0cc25d11f76d6..5a826dc9875bbe 100644
--- 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
index b3c9e75f6a0b63..7dfb8da370823e 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
@@ -99,7 +99,7 @@ - (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];
     }];
 }
@@ -107,7 +107,7 @@ - (oneway void)device:(NSNumber *)nodeID stateChanged:(MTRDeviceState)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];
     }];
 }
@@ -115,7 +115,7 @@ - (oneway void)device:(NSNumber *)nodeID receivedAttributeReport:(NSArray<NSDict
 - (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];
         }