Skip to content

Commit da17a3c

Browse files
bzbarsky-applewoody-apple
authored andcommitted
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 ddf6e2f commit da17a3c

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)