Skip to content

Commit 997932d

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 79da246 commit 997932d

File tree

1 file changed

+30
-11
lines changed

1 file changed

+30
-11
lines changed

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

+30-11
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ typedef NS_ENUM(NSUInteger, MTRDeviceWorkItemDuplicateTypeID) {
277277
#define MTRDEVICE_SUBSCRIPTION_LATENCY_NEW_VALUE_WEIGHT (1.0 / 3.0)
278278

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

333333
@property (nonatomic) BOOL expirationCheckScheduled;
334334

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

337337
@property (nonatomic) NSDate * estimatedStartTimeFromGeneralDiagnosticsUpTime;
338338

@@ -710,8 +710,17 @@ - (void)_setTimeOnDevice
710710

711711
- (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
712712
{
713+
os_unfair_lock_assert_owner(&self->_timeSyncLock);
714+
715+
auto timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.queue);
716+
717+
dispatch_source_set_timer(timerSource, dispatch_time(DISPATCH_TIME_NOW, nextUpdateInSeconds * NSEC_PER_SEC), DISPATCH_TIME_FOREVER,
718+
// Allow 3 seconds of leeway; should be plenty, in practice.
719+
static_cast<uint64_t>(3 * static_cast<double>(NSEC_PER_SEC)));
720+
713721
mtr_weakify(self);
714-
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (nextUpdateInSeconds * NSEC_PER_SEC)), self.queue, ^{
722+
dispatch_source_set_event_handler(timerSource, ^{
723+
dispatch_source_cancel(timerSource);
715724
mtr_strongify(self);
716725
MTR_LOG_DEBUG("%@ Timer expired, start Device Time Update", self);
717726
if (self) {
@@ -721,8 +730,9 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
721730
return;
722731
}
723732
});
724-
self.timeUpdateScheduled = YES;
733+
self.timeUpdateTimer = timerSource;
725734
MTR_LOG_DEBUG("%@ Timer Scheduled for next Device Time Update, in %llu seconds", self, nextUpdateInSeconds);
735+
dispatch_resume(timerSource);
726736
}
727737

728738
// Time Updates are a day apart (this can be changed in the future)
@@ -731,7 +741,7 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
731741
- (void)_updateDeviceTimeAndScheduleNextUpdate
732742
{
733743
os_unfair_lock_assert_owner(&self->_timeSyncLock);
734-
if (self.timeUpdateScheduled) {
744+
if (self.timeUpdateTimer != nil) {
735745
MTR_LOG_DEBUG("%@ Device Time Update already scheduled", self);
736746
return;
737747
}
@@ -749,14 +759,23 @@ - (void)_performScheduledTimeUpdate
749759
return;
750760
}
751761
// Device must not be invalidated
752-
if (!self.timeUpdateScheduled) {
762+
if (self.timeUpdateTimer == nil) {
753763
MTR_LOG_DEBUG("%@ Device Time Update is no longer scheduled, MTRDevice may have been invalidated.", self);
754764
return;
755765
}
756-
self.timeUpdateScheduled = NO;
766+
self.timeUpdateTimer = nil;
757767
[self _updateDeviceTimeAndScheduleNextUpdate];
758768
}
759769

770+
- (void)_cancelTimeUpdateTimer
771+
{
772+
std::lock_guard lock(self->_timeSyncLock);
773+
if (self.timeUpdateTimer != nil) {
774+
dispatch_source_cancel(self.timeUpdateTimer);
775+
self.timeUpdateTimer = nil;
776+
}
777+
}
778+
760779
- (NSArray<NSNumber *> *)_endpointsWithTimeSyncClusterServer
761780
{
762781
NSArray<NSNumber *> * endpointsOnDevice;
@@ -961,9 +980,7 @@ - (void)invalidate
961980

962981
[_asyncWorkQueue invalidate];
963982

964-
os_unfair_lock_lock(&self->_timeSyncLock);
965-
_timeUpdateScheduled = NO;
966-
os_unfair_lock_unlock(&self->_timeSyncLock);
983+
[self _cancelTimeUpdateTimer];
967984

968985
os_unfair_lock_lock(&self->_lock);
969986

@@ -1286,7 +1303,7 @@ - (void)_handleSubscriptionEstablished
12861303

12871304
os_unfair_lock_lock(&self->_timeSyncLock);
12881305

1289-
if (!self.timeUpdateScheduled) {
1306+
if (self.timeUpdateTimer == nil) {
12901307
[self _scheduleNextUpdate:MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC];
12911308
}
12921309

@@ -4758,6 +4775,8 @@ - (void)controllerSuspended
47584775
{
47594776
[super controllerSuspended];
47604777

4778+
[self _cancelTimeUpdateTimer];
4779+
47614780
std::lock_guard lock(self->_lock);
47624781
self.suspended = YES;
47634782
[self _resetSubscriptionWithReasonString:@"Controller suspended"];

0 commit comments

Comments
 (0)