SystemLayerImplSelect: libev: avoid timers firing early (#28434) (#28740

)

* SystemLayerImplSelect: libev: avoid timers firing slightly too early (#28434)

This fixes a ReportEngine problem that was caused by libev based timers
firing slightly (in the range of 20mS) too early, because libev by
default uses the time when events started processing as the "now"
reference for relative timers for efficiency reasons.

To ensure timers cannot fire early, timer setup must compensate for
any difference between ev_now() which is the time events started
processing and ev_time(), which is the actual current time (but is
a bit less efficient to obtain).

# Conflicts:
#	src/system/SystemLayerImplSelect.cpp

* SystemLayerImplSelect: libev: eliminate I/O wakeup thread

libev based regular builds do not need a separate I/O wakeup thread, so
the wakeup thread can be eliminated in CHIP_SYSTEM_CONFIG_USE_LIBEV case.

In normal operation, libev based builds also never call Signal(), so if it
is still called it now emits a error log message, to indicate something might
be wrong in the setup.

However we keep Signal() and related LayerSocketsLoop methods, following the
Darwin dispatch implementation, as fallback to select-based event handling
seems to be needed for some I/O tests.

As noted in a comment to the original libev PR [1], eventually, the select() based
mainloop and external mainloop based solutions like Darwin Dispatch and libev
should be detangled and extracted into separate classes, adapting all the tests
that somehow rely on the select() fallback. As a single self-funded
developer however, I cannot possibly be expected to solve this for Apple ;-)

[1] #24232 (review)

* SystemLayerImplSelect: reword comment: early firing timers can happen

- the fix prevents them in normal libev case
- however the caller MUST NOT rely on timers *never* firing a bit early

src/system/SystemLayerImplSelect.cpp

@@ -66,8 +66,10 @@ CHIP_ERROR LayerImplSelect::Init()
     mHandleSelectThread = PTHREAD_NULL;
 #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING
 
+#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
     // Create an event to allow an arbitrary thread to wake the thread in the select loop.
     ReturnErrorOnFailure(mWakeEvent.Open(*this));
+#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV
 
     VerifyOrReturnError(mLayerState.SetInitialized(), CHIP_ERROR_INCORRECT_STATE);
     return CHIP_NO_ERROR;
@@ -113,13 +115,18 @@ void LayerImplSelect::Shutdown()
     mTimerPool.ReleaseAll();
 #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV
 
+#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
     mWakeEvent.Close(*this);
+#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV
 
     mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization.
 }
 
 void LayerImplSelect::Signal()
 {
+#if CHIP_SYSTEM_CONFIG_USE_LIBEV
+    ChipLogError(DeviceLayer, "Signal() should not be called in CHIP_SYSTEM_CONFIG_USE_LIBEV builds (might be ok in tests)");
+#else
     /*
      * Wake up the I/O thread by writing a single byte to the wake pipe.
      *
@@ -143,6 +150,7 @@ void LayerImplSelect::Signal()
 
         ChipLogError(chipSystemLayer, "System wake event notify failed: %" CHIP_ERROR_FORMAT, status.Format());
     }
+#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV
 }
 
 CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
@@ -184,7 +192,14 @@ CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallba
     ev_timer_init(&timer->mLibEvTimer, &LayerImplSelect::HandleLibEvTimer, 1, 0);
     timer->mLibEvTimer.data = timer;
     auto t                  = Clock::Milliseconds64(delay).count();
-    ev_timer_set(&timer->mLibEvTimer, static_cast<double>(t) / 1E3, 0.);
+    // Note: libev uses the time when events started processing as the "now" reference for relative timers,
+    //   for efficiency reasons. This point in time is represented by ev_now().
+    //   The real time is represented by ev_time().
+    //   Without correction, this leads to timers firing a bit too early relative to the time StartTimer()
+    //   is called. So the relative value passed to ev_timer_set() is adjusted (increased) here.
+    // Note: Still, slightly early (and of course, late) firing timers are something the caller MUST be prepared for,
+    //   because edge cases like system clock adjustments may cause them even with the correction applied here.
+    ev_timer_set(&timer->mLibEvTimer, (static_cast<double>(t) / 1E3) + ev_time() - ev_now(mLibEvLoopP), 0.);
     (void) mTimerList.Add(timer);
     ev_timer_start(mLibEvLoopP, &timer->mLibEvTimer);
     return CHIP_NO_ERROR;
@@ -269,7 +284,10 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt
 #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV
 
     mTimerPool.Release(timer);
+#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
+    // LIBEV has no I/O wakeup thread, so must not call Signal()
     Signal();
+#endif
 }
 
 CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void * appState)

src/system/SystemLayerImplSelect.h

@@ -146,7 +146,9 @@ class LayerImplSelect : public LayerSocketsLoop
     int mSelectResult;
 
     ObjectLifeCycle mLayerState;
+#if !CHIP_SYSTEM_CONFIG_USE_LIBEV
     WakeEvent mWakeEvent;
+#endif
 
 #if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
     std::atomic<pthread_t> mHandleSelectThread;

src/system/WakeEvent.cpp

@@ -23,7 +23,7 @@
 
 #include <system/SystemConfig.h>
 
-#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
+#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV
 
 #include <system/WakeEvent.h>
 
@@ -207,4 +207,4 @@ CHIP_ERROR WakeEvent::Notify() const
 } // namespace System
 } // namespace chip
 
-#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
+#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV

src/system/WakeEvent.h

@@ -25,7 +25,7 @@
 // Include configuration headers
 #include <system/SystemConfig.h>
 
-#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
+#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV
 
 #include <lib/core/CHIPError.h>
 #include <system/SocketEvents.h>
@@ -67,4 +67,4 @@ class WakeEvent
 } // namespace System
 } // namespace chip
 
-#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
+#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV