Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Darwin] MTRDeviceController getSessionForNode should always use live MTRDevice object #34269

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,9 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
- (void)dealloc
{
[[NSNotificationCenter defaultCenter] removeObserver:_systemTimeChangeObserverToken];

// TODO: retain cycle and clean up https://github.com/project-chip/connectedhomeip/issues/34267
MTR_LOG("MTRDevice dealloc: %p", self);
}

- (NSString *)description
Expand Down
18 changes: 12 additions & 6 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
MTR_LOG("Loaded attribute values for %lu nodes from storage for controller uuid %@", static_cast<unsigned long>(clusterDataByNode.count), self->_uniqueIdentifier);

std::lock_guard lock(self->_deviceMapLock);
NSMutableArray * deviceList = [NSMutableArray array];
for (NSNumber * nodeID in clusterDataByNode) {
NSDictionary * clusterData = clusterDataByNode[nodeID];
MTRDevice * device = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:clusterData];
MTR_LOG("Loaded %lu cluster data from storage for %@", static_cast<unsigned long>(clusterData.count), device);

[deviceList addObject:device];
}

#define kSecondsToWaitBeforeAPIClientRetainsMTRDevice 60
// Keep the devices retained for a while, in case API client doesn't immedieately retain them
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (kSecondsToWaitBeforeAPIClientRetainsMTRDevice * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
MTR_LOG("MTRDeviceController: un-retain devices loaded at startup %lu", static_cast<unsigned long>(deviceList.count));
});
}];
}

Expand Down Expand Up @@ -1257,15 +1266,12 @@ - (BOOL)checkIsRunning:(NSError * __autoreleasing *)error

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

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