embassy-time: some driver_std fixes
#4978
Open
+20
−28
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I stumbled across a few oddities with the
embassy-timestd 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:schedule_wake(), it is blocked on the queue lockalarm_thread()wakes a waker that in itswake()enters a critical section=> fixed by wrapping the queue iteration in a critical section.
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 call
queue.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().