zephyr/machine_timer: Race condition calling machine_timer_deinit() from k_timer ISR context
Port, board and/or hardware
Zephyr port (all boards). The bug is in ports/zephyr/machine_timer.c which is shared across all Zephyr targets.
MicroPython version
MicroPython v1.27.0 (tag: v1.27.0)
Bug introduced in commit 3823aeb0f (Feb 2025) — "zephyr/machine_timer: Add machine.Timer class implementation."
Present in master as of v1.27.0.
Reproduction
machine_timer_callback() is called from Zephyr k_timer ISR context. For ONE_SHOT mode (or when mp_irq_dispatch fails), it calls machine_timer_deinit(), which traverses and modifies the MP_STATE_PORT(machine_timer_obj_head) linked list. The main thread may concurrently access the same list (GC, scheduler, other timer operations).
Minimal reproduction (run via mpremote exec, repeat if needed — the race is intermittent):
from machine import Timer
import time
def callback(t):
print("fired")
t = Timer(-1)
t.init(period=100, mode=Timer.ONE_SHOT, callback=callback)
time.sleep_ms(200)
print("done")
Stress test (more reliable trigger):
from machine import Timer
import time
for i in range(20):
t = Timer(-1)
t.init(period=10, mode=Timer.ONE_SHOT, callback=lambda x: None)
time.sleep_ms(50)
print("stress test passed")
Failure sequence:
- Timer fires after timeout in Zephyr
k_timerISR context - ISR calls
machine_timer_deinit()which traverses/modifies the linked list - Main thread in
time.sleep_ms()may be accessing the same list (GC, scheduler) - Race condition or deadlock occurs
- Board hangs,
mpremote exectimes out
The buggy code in ports/zephyr/machine_timer.c (lines 65–76):
static void machine_timer_callback(struct k_timer *timer) {
machine_timer_obj_t *self = (machine_timer_obj_t *)k_timer_user_data_get(timer);
if (mp_irq_dispatch(self->callback, MP_OBJ_FROM_PTR(self), self->ishard) < 0) {
self->mode = TIMER_MODE_ONE_SHOT;
}
if (self->mode == TIMER_MODE_ONE_SHOT) {
machine_timer_deinit(self); // BUG: linked list mutation from ISR
}
}
machine_timer_deinit() (lines 158–180) traverses and modifies MP_STATE_PORT(machine_timer_obj_head) — a linked list not protected against concurrent access from ISR and thread contexts.
Expected behaviour
The one-shot timer fires, the callback executes, and the program continues normally. Timer cleanup from ISR context should only perform ISR-safe operations (e.g. k_timer_stop()), not linked list manipulation.
Observed behaviour
The board intermittently hangs (deadlock/race condition). When run via mpremote exec, the command times out waiting for a response. The hang occurs because machine_timer_deinit() modifies the machine_timer_obj_head linked list from ISR context while the main thread may be traversing the same list.
Additional Information
Proposed fix: Replace machine_timer_deinit(self) with k_timer_stop(&self->my_timer) in the ISR callback. k_timer_stop() is ISR-safe per Zephyr's API guarantees. Linked list cleanup is deferred to thread context (GC, explicit deinit(), or machine_timer_deinit_all() on soft reset).
static void machine_timer_callback(struct k_timer *timer) {
machine_timer_obj_t *self = (machine_timer_obj_t *)k_timer_user_data_get(timer);
if (self->mode == TIMER_MODE_ONE_SHOT) {
k_timer_stop(&self->my_timer);
}
if (mp_irq_dispatch(self->callback, MP_OBJ_FROM_PTR(self), self->ishard) < 0) {
self->mode = TIMER_MODE_ONE_SHOT;
k_timer_stop(&self->my_timer);
}
}
I can submit a PR with this fix if the approach is acceptable.
Code of Conduct
- Yes, I agree to follow the MicroPython Code of Conduct
RFC: defining machine.Timer
This issue is to discuss the API of the machine.Timer class. There are currently 3 different version of Timer which don't have much in common:
esp8266:
# id must be -1, period is in milliseconds, mode is PERIODIC or ONE_SHOT
Timer(id, *, period, mode, callback) # constructor
Timer.init(*, period, mode, callback) # reinitialise all parameters
Timer.deinit() # stop the timer
cc3200/wipy:
# mode is PERIODIC, ONE_SHOT or PWM
Timer(id, mode, *, width=16) # constructor
Timer.init(mode, *, width=16) # reinitialise
Timer.deinit() # stop
# each timer has one or more channels
# each timer channel is associated with a specific IO pin (eg for PWM)
Timer.channel(*, freq, period, polarity, duty_cycle) # create channel assoc with TImer
TimerChannel.freq([freq]) # get/set
TimerChannel.period([period]) # get/set
TimerChannel.duty_cycle([duty]) # get/set
TimerChannel.irq(...) # get or set up the IRQ callback
stmhal/pyboard (actually pyb.Timer):
# freq is in Hz (can be float <1), prescaler/period are raw values, mode is UP/DOWN/CENTER
Timer(id, *, freq, prescaler, period, mode, div, callback, deadtime) # constructor
Timer.init(*, freq, prescaler, period, mode, div, callback, deadtime) # reinit
Timer.counter([value]) # get/set the raw timer counter
Timer.source_freq() # get source frequency that clocks this timer
Timer.freq([value]) # get/set
Timer.prescaler([value]) # get/set
Timer.period([value]) # get/set
Timer.callback(callable) # get/set callback (pass None to disable)
# each timer has multiple chanels (usually 4)
# pin is the pin to associate with the channel (eg for PWM)
Timer.channel(mode, *, callback, pin, pulse_width, pulse_width_percent, compare, polarity) # create
TimerChannel.pulse_width([value]) # get/set
TimerChannel.pulse_width_percent([value]) # get/set
TimerChannel.capture([value]) # get/set
TimerChannel.compare([value]) # get/set
TimerChannel.callback(callable) # get/set callback
To compare current usage, for a simple periodic timer at 10Hz one would do:
# esp8266
t = Timer(-1)
t.init(period=100, mode=Timer.PERIODIC, callback=lambda t:print(t))
# stmhal/pyboard
Timer(1, freq=10, callback=lambda t:print(t))
For a one-shot after 2 seconds:
# esp8266
t = Timer(-1)
t.init(period=2000, mode=Timer.ONE_SHOT, callback=lambda t:print(t))
# stmhal/pyboard
def cb(t):
print(t)
t.deinit() # or t.callback(None)
Timer(1, freq=0.5, callback=cb)
At the very least I'd suggest that all ports have the same interface to create periodic and one-shot timers, as well as the same way to configure the timer irq/callback. Configuration of the irq should follow the samy signature as machine.Pin.irq(); see also #2297.
I also feel that a timer should have start() and stop() methods, so that one can create an initialise a timer at one point in the code, then start it precisely when needed. This requires definining that the first callback happens one period after start() is called.