Skip to content

Commit 40979b4

Browse files
[Darwin] MTRDevice delegate callbacks should call out without holding lock (#37668)
* [Darwin] MTRDevice delegate callbacks should call out without holding lock * Fixed compilation issues * Update src/darwin/Framework/CHIP/MTRDevice_XPC.mm Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDevice_XPC.mm Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Change back to lock_guard --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 8464e5f commit 40979b4

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm

+15-16
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#import "MTRDevice_XPC_Internal.h"
2424
#import "MTRError_Internal.h"
2525
#import "MTRLogging_Internal.h"
26+
#import "MTRUnfairLock.h"
2627
#import "MTRXPCClientProtocol.h"
2728
#import "MTRXPCServerProtocol.h"
2829

@@ -72,28 +73,26 @@ @implementation MTRDeviceController_XPC
7273

7374
- (void)_updateRegistrationInfo
7475
{
75-
dispatch_async(self.workQueue, ^{
76-
std::lock_guard lock(*self.deviceMapLock);
76+
std::lock_guard lock(*self.deviceMapLock);
7777

78-
NSMutableDictionary * registrationInfo = [NSMutableDictionary dictionary];
78+
NSMutableDictionary * registrationInfo = [NSMutableDictionary dictionary];
7979

80-
NSMutableDictionary * controllerContext = [NSMutableDictionary dictionary];
81-
NSMutableArray * nodeIDs = [NSMutableArray array];
80+
NSMutableDictionary * controllerContext = [NSMutableDictionary dictionary];
81+
NSMutableArray * nodeIDs = [NSMutableArray array];
8282

83-
for (NSNumber * nodeID in [self.nodeIDToDeviceMap keyEnumerator]) {
84-
MTRDevice * device = self.nodeIDToDeviceMap[nodeID];
85-
if ([device delegateExists]) {
86-
NSMutableDictionary * nodeDictionary = [NSMutableDictionary dictionary];
87-
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationNodeIDKey, nodeID, nodeDictionary)
83+
for (NSNumber * nodeID in [self.nodeIDToDeviceMap keyEnumerator]) {
84+
MTRDevice * device = [self.nodeIDToDeviceMap objectForKey:nodeID];
85+
if ([device delegateExists]) {
86+
NSMutableDictionary * nodeDictionary = [NSMutableDictionary dictionary];
87+
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationNodeIDKey, nodeID, nodeDictionary)
8888

89-
[nodeIDs addObject:nodeDictionary];
90-
}
89+
[nodeIDs addObject:nodeDictionary];
9190
}
92-
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationNodeIDsKey, nodeIDs, registrationInfo)
93-
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationControllerContextKey, controllerContext, registrationInfo)
91+
}
92+
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationNodeIDsKey, nodeIDs, registrationInfo)
93+
MTR_REQUIRED_ATTRIBUTE(MTRDeviceControllerRegistrationControllerContextKey, controllerContext, registrationInfo)
9494

95-
[self updateControllerConfiguration:registrationInfo];
96-
});
95+
[self updateControllerConfiguration:registrationInfo];
9796
}
9897

9998
- (void)_registerNodeID:(NSNumber *)nodeID

src/darwin/Framework/CHIP/MTRDevice_XPC.mm

+14-2
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,28 @@ - (MTRNetworkCommissioningFeature)networkCommissioningFeatures
151151

152152
- (void)_delegateAdded:(id<MTRDeviceDelegate>)delegate
153153
{
154+
os_unfair_lock_assert_owner(&self->_lock);
155+
154156
[super _delegateAdded:delegate];
155157
MTR_LOG("%@ delegate added: %@", self, delegate);
156-
[(MTRDeviceController_XPC *) [self deviceController] _updateRegistrationInfo];
158+
159+
// dispatch to own queue so we're not holding our lock while calling out to the controller code.
160+
dispatch_async(self.queue, ^{
161+
[(MTRDeviceController_XPC *) [self deviceController] _updateRegistrationInfo];
162+
});
157163
}
158164

159165
- (void)_delegateRemoved:(id<MTRDeviceDelegate>)delegate
160166
{
167+
os_unfair_lock_assert_owner(&self->_lock);
168+
161169
[super _delegateRemoved:delegate];
162170
MTR_LOG("%@ delegate removed: %@", self, delegate);
163-
[(MTRDeviceController_XPC *) [self deviceController] _updateRegistrationInfo];
171+
172+
// dispatch to own queue so we're not holding our lock while calling out to the controller code.
173+
dispatch_async(self.queue, ^{
174+
[(MTRDeviceController_XPC *) [self deviceController] _updateRegistrationInfo];
175+
});
164176
}
165177

166178
#pragma mark - Client Callbacks (MTRDeviceDelegate)

0 commit comments

Comments
 (0)