From 6d1d924ff91dbfe05c81c4cfbfcae10f89ab01eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 06:28:01 -0500 Subject: [PATCH] [core] time_64 NO_ATOMICS: atomic re-reads + reload major under lock Two fixes Copilot flagged on #15947: 1. Use __atomic_load_n to re-read last_millis under the lock, not a plain read. The forward-progression branch (else if) writes last_millis with __atomic_store_n without holding lock, so the under-lock plain read would race with it and be UB in the C++ memory model. 2. Reload major from millis_major after acquiring the lock. The unlocked load at the top of the function can be stale by the time we get the lock: another thread may have completed a rollover between the unlocked load and the lock acquisition, leaving our local major behind by one. Without reload the function could return a 64-bit timestamp that jumps backwards by ~2^32 ms (~49.7 days). The MULTI_ATOMICS branch already handles this via its retry loop; NO_ATOMICS just reloads under the lock. --- esphome/core/time_64.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/esphome/core/time_64.cpp b/esphome/core/time_64.cpp index d9a12aa477..c18ccd44f8 100644 --- a/esphome/core/time_64.cpp +++ b/esphome/core/time_64.cpp @@ -87,8 +87,13 @@ uint64_t Millis64Impl::compute(uint32_t now) { if (near_rollover || (now < last && (last - now) > HALF_MAX_UINT32)) { // Near rollover or detected a rollover - need lock for safety LockGuard guard{lock}; - // Re-read with lock held - last = last_millis; + // Re-read both values with lock held. last_millis can be updated + // unlocked from the forward-progression branch below, so use an atomic + // load. millis_major can only be updated under this lock, but another + // thread may have completed a rollover between our unlocked loads above + // and the lock acquisition — reload or we'd return a stale high word. + last = __atomic_load_n(&last_millis, __ATOMIC_RELAXED); + major = __atomic_load_n(&millis_major, __ATOMIC_RELAXED); if (now < last && (last - now) > HALF_MAX_UINT32) { // True rollover detected (happens every ~49.7 days)