embassy-time: some `driver_std` fixes
I stumbled across a few oddities with the embassy-time std driver:
-
Both
now()andschedule_wake()were lockinginner. That made any use ofnow()within a waker cause a deadlock.=> fixed by splitting
innerinto its fieldsqueueandzero_instant, and usingLazyLockfor initialization. -
The
alarm_thread()iterates the timer queue while holding the queue lock, but outside a critical section. Now if:- another thread holds a critical section and calls
schedule_wake(), it is blocked on the queue lock alarm_thread()wakes a waker that in itswake()enters a critical section- the two threads dead-lock
=> fixed by wrapping the queue iteration in a critical section.
- another thread holds a critical section and calls
-
Previously,
schedule_wake()would just pass theWakerto the queue'sschedule_wake()and signal the alarm thread. If two consecutiveWakersare scheduled, the first beforenowand the second afternow, only the first would get woken (as soon asalarm_thread()processes the signal). This behaves differently than all the other time driver implementations, that all callqueue.next_iteration()withinschedule_wake()and thus execute already passed timeouts right away, causing two wakeups.While I believe consistent behavior alone justifies this change, it is also necessary for using the
embassy-timemachinery outside of tasks, because aWakercannot re-schedule itself to a later timeout in itswake(), as callingschedule_wake()while the queue lock is held causes a deadlock. So a waker can only check if it should wake (throughnow()and its context). Through first setting it withat=0(already passed), whichwakes()once and then drops the waker from the queue, it can be set again with anyat.=> fixed by also calling
queue.next_iteration()withinschedule_wake().
I do see this is basically fixing three things, should I split this up? Or would that triple testing?
we should avoid using critical_section in code targeting std. It's basically a global mutex, and using real mutexes allows for finer grained locking.