Skip to content

Commit 75b5405

Browse files
Cancel the periodic Time Sync timer in Matter.framework when the controller is suspended. (#37230)
We shouldn't be trying to talk to the device in that state.
1 parent 0e47155 commit 75b5405

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)