From 58420de330a6f4bbbccccc27e1a72ed505fa9a4c Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 25 May 2023 22:30:01 -0400 Subject: [PATCH] Fix ordering issue with subscribeToAttributesWithEndpointID over XPC. (#26850) We had an extra queue hop for reports compared with the subscriptionEstablished notification, so the API consumer could get subscriptionEstablished before getting all the priming reports. The fix is to just ensure that the code flows the same way, in terms of queue dispatch, for both priming reports and subscriptionEstablished. --- .../CHIP/MTRDeviceControllerXPCConnection.h | 3 ++- .../CHIP/MTRDeviceControllerXPCConnection.mm | 7 +++++++ src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm | 18 ++++++++++-------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.h b/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.h index 3c6a82c1235ec0..8660a4fbb07a10 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.h @@ -43,7 +43,7 @@ typedef void (^MTRGetProxyHandleHandler)(dispatch_queue_t queue, MTRDeviceContro @interface MTRDeviceControllerXPCConnection : NSObject /** - * This method is just for test purpsoe. + * This method is just for test purpose. */ + (MTRDeviceControllerXPCConnection *)connectionWithWorkQueue:(dispatch_queue_t)workQueue connectBlock:(MTRXPCConnectBlock)connectBlock; @@ -55,6 +55,7 @@ typedef void (^MTRGetProxyHandleHandler)(dispatch_queue_t queue, MTRDeviceContro - (void)deregisterReportHandlersWithController:(id)controller nodeID:(NSNumber *)nodeID completion:(dispatch_block_t)completion; +- (void)callSubscriptionEstablishedHandler:(dispatch_block_t)handler; @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.mm index 9620f4ba757339..e44308244cc00f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.mm @@ -203,4 +203,11 @@ - (void)handleReportWithController:(id)controller }); } +- (void)callSubscriptionEstablishedHandler:(dispatch_block_t)handler +{ + // Call the handler from our _workQueue, so that we guarantee the same + // number of queue hops as for handleReportWithController. + dispatch_async(_workQueue, handler); +} + @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm index 17cd039278d294..cdc2dacb20c37c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm @@ -272,14 +272,16 @@ - (void)subscribeToAttributesWithEndpointID:(NSNumber * _Nullable)endpointID maxInterval:params.maxInterval params:[MTRDeviceController encodeXPCSubscribeParams:params] establishedHandler:^{ - dispatch_async(queue, ^{ - MTR_LOG_DEBUG("Subscription established"); - subscriptionEstablishedHandler(); - // The following captures the proxy handle in the closure so that the handle - // won't be released prior to block call. - __auto_type handleRetainer = handle; - (void) handleRetainer; - }); + [self.xpcConnection callSubscriptionEstablishedHandler:^{ + dispatch_async(queue, ^{ + MTR_LOG_DEBUG("Subscription established"); + subscriptionEstablishedHandler(); + // The following captures the proxy handle in the closure so that the handle + // won't be released prior to block call. + __auto_type handleRetainer = handle; + (void) handleRetainer; + }); + }]; }]; };