Skip to content

Commit d38ab9a

Browse files
jtung-applewoody-applebzbarsky-apple
authored
[Darwin] MTRDeviceController getSessionForNode should always use live MTRDevice object (#34269)
* [Darwin] MTRDeviceController should always examine the MTRDevice before using subscription/CASE pool * Update src/darwin/Framework/CHIP/MTRDevice.mm Co-authored-by: Justin Wood <woody@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceController.mm Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Justin Wood <woody@apple.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 171843d commit d38ab9a

File tree

2 files changed

+18
-6
lines changed

2 files changed

+18
-6
lines changed

src/darwin/Framework/CHIP/MTRDevice.mm

+3
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,9 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
539539
- (void)dealloc
540540
{
541541
[[NSNotificationCenter defaultCenter] removeObserver:_systemTimeChangeObserverToken];
542+
543+
// TODO: retain cycle and clean up https://github.com/project-chip/connectedhomeip/issues/34267
544+
MTR_LOG("MTRDevice dealloc: %p", self);
542545
}
543546

544547
- (NSString *)description

src/darwin/Framework/CHIP/MTRDeviceController.mm

+15-6
Original file line numberDiff line numberDiff line change
@@ -605,11 +605,23 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
605605
MTR_LOG("Loaded attribute values for %lu nodes from storage for controller uuid %@", static_cast<unsigned long>(clusterDataByNode.count), self->_uniqueIdentifier);
606606

607607
std::lock_guard lock(self->_deviceMapLock);
608+
NSMutableArray * deviceList = [NSMutableArray array];
608609
for (NSNumber * nodeID in clusterDataByNode) {
609610
NSDictionary * clusterData = clusterDataByNode[nodeID];
610611
MTRDevice * device = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:clusterData];
611612
MTR_LOG("Loaded %lu cluster data from storage for %@", static_cast<unsigned long>(clusterData.count), device);
613+
614+
[deviceList addObject:device];
612615
}
616+
617+
#define kSecondsToWaitBeforeAPIClientRetainsMTRDevice 60
618+
// Keep the devices retained for a while, in case API client doesn't immediately retain them.
619+
//
620+
// Note that this is just an optimization to avoid throwing the information away and immediately
621+
// re-reading it from storage.
622+
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (kSecondsToWaitBeforeAPIClientRetainsMTRDevice * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
623+
MTR_LOG("MTRDeviceController: un-retain devices loaded at startup %lu", static_cast<unsigned long>(deviceList.count));
624+
});
613625
}];
614626
}
615627

@@ -1257,15 +1269,12 @@ - (BOOL)checkIsRunning:(NSError * __autoreleasing *)error
12571269

12581270
- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
12591271
{
1260-
// First check if MTRDevice exists from having loaded from storage, or created by a client.
1261-
// Do not use deviceForNodeID here, because we don't want to create the device if it does not already exist.
1262-
os_unfair_lock_lock(&_deviceMapLock);
1263-
MTRDevice * device = [_nodeIDToDeviceMap objectForKey:@(nodeID)];
1264-
os_unfair_lock_unlock(&_deviceMapLock);
1272+
// Get the corresponding MTRDevice object to determine if the case/subscription pool is to be used
1273+
MTRDevice * device = [self deviceForNodeID:@(nodeID)];
12651274

12661275
// In the case that this device is known to use thread, queue this with subscription attempts as well, to
12671276
// help with throttling Thread traffic.
1268-
if (device && [device deviceUsesThread]) {
1277+
if ([device deviceUsesThread]) {
12691278
MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)];
12701279
[workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull workItemCompletion) {
12711280
MTRInternalDeviceConnectionCallback completionWrapper = ^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,

0 commit comments

Comments
 (0)