mirror of
https://github.com/esphome/esphome.git
synced 2026-07-10 08:55:36 +00:00
[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.
This commit is contained in:
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user