Skip to content

Commit e1e4fd3

Browse files
Cancel the periodic Time Sync timer in Matter.framework when the controller is suspended.
We shouldn't be trying to talk to the device in that state.
1 parent 0c1a606 commit e1e4fd3

File tree

1 file changed

+29
-11
lines changed

1 file changed

+29
-11
lines changed

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

+29-11
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ typedef NS_ENUM(NSUInteger, MTRDeviceWorkItemDuplicateTypeID) {
212212
#define MTRDEVICE_SUBSCRIPTION_LATENCY_NEW_VALUE_WEIGHT (1.0 / 3.0)
213213

214214
@interface MTRDevice_Concrete ()
215-
// protects against concurrent time updates by guarding timeUpdateScheduled flag which manages time updates scheduling,
215+
// protects against concurrent time updates by guarding the timeUpdateTimer field, which manages time update scheduling,
216216
// and protects device calls to setUTCTime and setDSTOffset. This can't just be replaced with "lock", because the time
217217
// update code calls public APIs like readAttributeWithEndpointID:.. (which attempt to take "lock") while holding
218218
// whatever lock protects the time sync bits.
@@ -266,7 +266,7 @@ @interface MTRDevice_Concrete ()
266266

267267
@property (nonatomic) BOOL expirationCheckScheduled;
268268

269-
@property (nonatomic) BOOL timeUpdateScheduled;
269+
@property (nonatomic, retain, readwrite, nullable) dispatch_source_t timeUpdateTimer;
270270

271271
@property (nonatomic) NSDate * estimatedStartTimeFromGeneralDiagnosticsUpTime;
272272

@@ -607,8 +607,17 @@ - (void)_setTimeOnDevice
607607

608608
- (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
609609
{
610+
os_unfair_lock_assert_owner(&self->_timeSyncLock);
611+
612+
auto timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.queue);
613+
614+
dispatch_source_set_timer(timerSource, dispatch_time(DISPATCH_TIME_NOW, nextUpdateInSeconds * NSEC_PER_SEC), DISPATCH_TIME_FOREVER,
615+
// Allow 3 seconds of leeway; should be plenty, in practice.
616+
static_cast<uint64_t>(3 * static_cast<double>(NSEC_PER_SEC)));
617+
610618
mtr_weakify(self);
611-
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (nextUpdateInSeconds * NSEC_PER_SEC)), self.queue, ^{
619+
dispatch_source_set_event_handler(timerSource, ^{
620+
dispatch_source_cancel(timerSource);
612621
MTR_LOG_DEBUG("%@ Timer expired, start Device Time Update", self);
613622
mtr_strongify(self);
614623
if (self) {
@@ -618,7 +627,7 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
618627
return;
619628
}
620629
});
621-
self.timeUpdateScheduled = YES;
630+
self.timeUpdateTimer = timerSource;
622631
MTR_LOG_DEBUG("%@ Timer Scheduled for next Device Time Update, in %llu seconds", self, nextUpdateInSeconds);
623632
}
624633

@@ -628,7 +637,7 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
628637
- (void)_updateDeviceTimeAndScheduleNextUpdate
629638
{
630639
os_unfair_lock_assert_owner(&self->_timeSyncLock);
631-
if (self.timeUpdateScheduled) {
640+
if (self.timeUpdateTimer != nil) {
632641
MTR_LOG_DEBUG("%@ Device Time Update already scheduled", self);
633642
return;
634643
}
@@ -646,14 +655,23 @@ - (void)_performScheduledTimeUpdate
646655
return;
647656
}
648657
// Device must not be invalidated
649-
if (!self.timeUpdateScheduled) {
658+
if (self.timeUpdateTimer == nil) {
650659
MTR_LOG_DEBUG("%@ Device Time Update is no longer scheduled, MTRDevice may have been invalidated.", self);
651660
return;
652661
}
653-
self.timeUpdateScheduled = NO;
662+
self.timeUpdateTimer = nil;
654663
[self _updateDeviceTimeAndScheduleNextUpdate];
655664
}
656665

666+
- (void)_cancelTimeUpdateTimer
667+
{
668+
std::lock_guard lock(self->_timeSyncLock);
669+
if (self.timeUpdateTimer != nil) {
670+
dispatch_source_cancel(self.timeUpdateTimer);
671+
self.timeUpdateTimer = nil;
672+
}
673+
}
674+
657675
- (NSArray<NSNumber *> *)_endpointsWithTimeSyncClusterServer
658676
{
659677
NSArray<NSNumber *> * endpointsOnDevice;
@@ -844,9 +862,7 @@ - (void)invalidate
844862

845863
[_asyncWorkQueue invalidate];
846864

847-
os_unfair_lock_lock(&self->_timeSyncLock);
848-
_timeUpdateScheduled = NO;
849-
os_unfair_lock_unlock(&self->_timeSyncLock);
865+
[self _cancelTimeUpdateTimer];
850866

851867
os_unfair_lock_lock(&self->_lock);
852868

@@ -1143,7 +1159,7 @@ - (void)_handleSubscriptionEstablished
11431159

11441160
os_unfair_lock_lock(&self->_timeSyncLock);
11451161

1146-
if (!self.timeUpdateScheduled) {
1162+
if (self.timeUpdateTimer == nil) {
11471163
[self _scheduleNextUpdate:MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC];
11481164
}
11491165

@@ -4285,6 +4301,8 @@ - (void)controllerSuspended
42854301
{
42864302
[super controllerSuspended];
42874303

4304+
[self _cancelTimeUpdateTimer];
4305+
42884306
std::lock_guard lock(self->_lock);
42894307
self.suspended = YES;
42904308
[self _resetSubscriptionWithReasonString:@"Controller suspended"];

0 commit comments

Comments
 (0)