From 3ffd88a84acdfc585767cced9b703ed544789590 Mon Sep 17 00:00:00 2001
From: Maisem Ali <maisem@tailscale.com>
Date: Sun, 17 Apr 2022 21:36:35 -0700
Subject: [PATCH] wgengine/monitor: do not set timeJumped on iOS/Android

In `(*Mon).Start` we don't run a timer to update `(*Mon).lastWall` on iOS and
Android as their sleep patterns are bespoke. However, in the debounce
goroutine we would notice that the the wall clock hadn't been updated
since the last event would assume that a time jump had occurred. This would
result in non-events being considered as major-change events.

This commit makes it so that `(*Mon).timeJumped` is never set to `true`
on iOS and Android.

Signed-off-by: Maisem Ali <maisem@tailscale.com>
---
 wgengine/monitor/monitor.go | 59 +++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/wgengine/monitor/monitor.go b/wgengine/monitor/monitor.go
index f5e3f64d6..d2ddfb8c3 100644
--- a/wgengine/monitor/monitor.go
+++ b/wgengine/monitor/monitor.go
@@ -185,13 +185,7 @@ func (m *Mon) Start() {
 	}
 	m.started = true
 
-	switch runtime.GOOS {
-	case "ios", "android":
-		// For battery reasons, and because these platforms
-		// don't really sleep in the same way, don't poll
-		// for the wall time to detect for wake-for-sleep
-		// walltime jumps.
-	default:
+	if shouldMonitorTimeJump {
 		m.wallTimer = time.AfterFunc(pollWallTimeInterval, m.pollWallTime)
 	}
 
@@ -304,16 +298,9 @@ func (m *Mon) debounce() {
 		} else {
 			m.mu.Lock()
 
-			// See if we have a queued or new time jump signal.
-			m.checkWallTimeAdvanceLocked()
-			timeJumped := m.timeJumped
-			if timeJumped {
-				m.logf("time jumped (probably wake from sleep); synthesizing major change event")
-			}
-
 			oldState := m.ifState
-			ifChanged := !curState.EqualFiltered(oldState, interfaces.UseInterestingInterfaces, interfaces.UseInterestingIPs)
-			if ifChanged {
+			changed := !curState.EqualFiltered(oldState, interfaces.UseInterestingInterfaces, interfaces.UseInterestingIPs)
+			if changed {
 				m.gwValid = false
 				m.ifState = curState
 
@@ -322,9 +309,14 @@ func (m *Mon) debounce() {
 						jsonSummary(oldState), jsonSummary(curState))
 				}
 			}
-			changed := ifChanged || timeJumped
-			if changed {
-				m.timeJumped = false
+			// See if we have a queued or new time jump signal.
+			if shouldMonitorTimeJump && m.checkWallTimeAdvanceLocked() {
+				m.resetTimeJumpedLocked()
+				if !changed {
+					// Only log if it wasn't an interesting change.
+					m.logf("time jumped (probably wake from sleep); synthesizing major change event")
+					changed = true
+				}
 			}
 			for _, cb := range m.cbs {
 				go cb(changed, m.ifState)
@@ -360,22 +352,37 @@ func (m *Mon) pollWallTime() {
 	if m.closed {
 		return
 	}
-	m.checkWallTimeAdvanceLocked()
-	if m.timeJumped {
+	if m.checkWallTimeAdvanceLocked() {
 		m.InjectEvent()
 	}
 	m.wallTimer.Reset(pollWallTimeInterval)
 }
 
-// checkWallTimeAdvanceLocked updates m.timeJumped, if wall time jumped
-// more than 150% of pollWallTimeInterval, indicating we probably just
-// came out of sleep.
-func (m *Mon) checkWallTimeAdvanceLocked() {
+// shouldMonitorTimeJump is whether we keep a regular periodic timer running in
+// the background watching for jumps in wall time.
+//
+// We don't do this on mobile platforms for battery reasons, and because these
+// platforms don't really sleep in the same way.
+const shouldMonitorTimeJump = runtime.GOOS != "android" && runtime.GOOS != "ios"
+
+// checkWallTimeAdvanceLocked reports whether wall time jumped more than 150% of
+// pollWallTimeInterval, indicating we probably just came out of sleep. Once a
+// time jump is detected it must be reset by calling resetTimeJumpedLocked.
+func (m *Mon) checkWallTimeAdvanceLocked() bool {
+	if !shouldMonitorTimeJump {
+		panic("unreachable") // if callers are correct
+	}
 	now := wallTime()
 	if now.Sub(m.lastWall) > pollWallTimeInterval*3/2 {
-		m.timeJumped = true
+		m.timeJumped = true // it is reset by debounce.
 	}
 	m.lastWall = now
+	return m.timeJumped
+}
+
+// resetTimeJumpedLocked consumes the signal set by checkWallTimeAdvanceLocked.
+func (m *Mon) resetTimeJumpedLocked() {
+	m.timeJumped = false
 }
 
 type ipRuleDeletedMessage struct {