fix: k_timer_status_sync blocks on an embedded sem, not a poll loop (#28)

components/zkernel/include/boreas/zephyr/kernel.h

@@ -182,18 +182,24 @@ struct k_sem {
 	portMUX_TYPE lock;
 };
 
+/* Compile-time k_sem initializer body. @p self must be the sem being
+ * initialized (the waiters list is self-referential). Single source
+ * for K_SEM_DEFINE and embedded sems (K_TIMER_DEFINE). */
+#define Z_SEM_INITIALIZER(self, _initial, _limit)                                                  \
+	{                                                                                          \
+		.count = (_initial),                                                               \
+		.limit = (_limit),                                                                 \
+		.waiters = SYS_DLIST_STATIC_INIT(&(self).waiters),                                 \
+		.lock = portMUX_INITIALIZER_UNLOCKED,                                              \
+	}
+
 /**
  * Statically define a fully-initialized semaphore. True compile-time
  * initializer (matches upstream Zephyr): usable from constructors and
  * SYS_INIT callbacks without any runtime init step.
  */
 #define K_SEM_DEFINE(name, _initial, _limit)                                                       \
-	struct k_sem name = {                                                                      \
-		.count = (_initial),                                                               \
-		.limit = (_limit),                                                                 \
-		.waiters = SYS_DLIST_STATIC_INIT(&name.waiters),                                   \
-		.lock = portMUX_INITIALIZER_UNLOCKED,                                              \
-	};                                                                                         \
+	struct k_sem name = Z_SEM_INITIALIZER(name, _initial, _limit);                             \
 	BUILD_ASSERT(((_limit) != 0) && ((_initial) <= (_limit)),                                  \
 		     "K_SEM_DEFINE: limit must be nonzero and >= initial") /* upstream parity */
 
@@ -415,6 +421,10 @@ struct k_timer {
 	k_timer_stop_t stop_fn;
 	void *user_data;
 	uint32_t status; /* expiry count since last status read */
+	/* status_sync wake latch (binary sem, upstream's single-waiter
+	 * model): given after each expiry and by k_timer_stop. The COUNT
+	 * lives in `status`; the sem only carries the wake. */
+	struct k_sem sync_sem;
 	bool running;
 	bool is_periodic;            /* last k_timer_start was periodic; one-shot if false */
 	bool first_interval_pending; /* start-once then switch to periodic */
@@ -429,8 +439,15 @@ struct k_timer {
 	struct k_timer name = {                                                                    \
 		.expiry_fn = (_expiry_fn),                                                         \
 		.stop_fn = (_stop_fn),                                                             \
+		.sync_sem = Z_SEM_INITIALIZER(name.sync_sem, 0, 1),                                \
 	}
 
+/**
+ * @note Re-initializing a timer while a thread is blocked in
+ *       k_timer_status_sync on it is a caller error (it clobbers the
+ *       embedded semaphore's waiter list) -- the same reuse rule as
+ *       re-initializing a k_sem with a blocked taker.
+ */
 void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_stop_t stop_fn);
 /**
  * @brief Start or restart a timer.
@@ -445,6 +462,10 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto
  *          synchronize with an in-flight ISR callback on SMP targets.
  *          On linux, k_timer_stop likewise does not synchronize with a
  *          callback the dispatcher task has already dequeued.
+ *
+ * @note Restarting does NOT wake a thread blocked in
+ *       k_timer_status_sync -- it keeps waiting for the restarted
+ *       timer's first expiry (or a stop), matching upstream.
  */
 void k_timer_start(struct k_timer *timer, k_timeout_t duration, k_timeout_t period);
 void k_timer_stop(struct k_timer *timer);
@@ -460,15 +481,16 @@ uint32_t k_timer_status_get(struct k_timer *timer);
  * Returns immediately if the count is already non-zero, or if the
  * timer is already stopped.
  *
- * @note Must not be called from an interrupt handler (blocks).
+ * @note Must not be called from an interrupt handler (blocks) --
+ *       matches upstream.
  *
- * @note Boreas implementation polls every k_msleep(1). Upstream
- *       Zephyr blocks the calling thread on a wait queue. Same
- *       caller-visible semantics; the wake granularity here is
- *       bounded by the FreeRTOS tick period (portTICK_PERIOD_MS,
- *       set by CONFIG_FREERTOS_HZ -- 1ms at the Boreas default of
- *       1000 Hz, 10ms at the ESP-IDF Kconfig default of 100 Hz).
- *       A wake can therefore be up to one tick period late.
+ * @note Blocks on the timer's embedded semaphore, woken by expiry or
+ *       k_timer_stop (upstream's single-waiter wait-queue model: one
+ *       thread waits per timer).
+ * @note Divergence: a thread blocked here must NOT be aborted.
+ *       Upstream unpends an aborted thread from the timer wait queue;
+ *       Boreas cannot, so the dead thread would leave a dangling
+ *       waiter node (same limitation as k_sem_take -- see its @note).
  */
 uint32_t k_timer_status_sync(struct k_timer *timer);
 

components/zkernel/src/k_timer.c

@@ -40,6 +40,11 @@ static void K_ISR_SAFE k_timer_esp_callback(void *arg)
 	if (!__atomic_load_n(&timer->is_periodic, __ATOMIC_ACQUIRE)) {
 		__atomic_store_n(&timer->running, false, __ATOMIC_RELEASE);
 	}
+
+	/* Wake a status_sync waiter LAST (upstream wakes at the end of
+	 * its expiration handler, after the expiry callback). ISR-safe
+	 * under both dispatch modes. */
+	k_sem_give(&timer->sync_sem);
 }
 
 void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_stop_t stop_fn)
@@ -53,6 +58,9 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto
 	timer->first_interval_pending = false;
 	timer->period_us = 0;
 	timer->handle = NULL;
+	/* Binary: the sem is the status_sync wake latch, not a counter
+	 * (the expiry count lives in timer->status). */
+	(void)k_sem_init(&timer->sync_sem, 0, 1);
 
 	const esp_timer_create_args_t args = {
 		.callback = k_timer_esp_callback,
@@ -133,6 +141,9 @@ void k_timer_stop(struct k_timer *timer)
 		if (timer->stop_fn) {
 			timer->stop_fn(timer);
 		}
+		/* Wake a status_sync waiter (upstream unpends one waiter on
+		 * stop, after invoking the stop function). */
+		k_sem_give(&timer->sync_sem);
 	}
 }
 
@@ -143,18 +154,32 @@ uint32_t k_timer_status_get(struct k_timer *timer)
 
 uint32_t k_timer_status_sync(struct k_timer *timer)
 {
-	/* Block until the timer expires or is stopped. Polls every
-	 * k_msleep(1), which rounds up to one FreeRTOS tick
-	 * (portTICK_PERIOD_MS, set by CONFIG_FREERTOS_HZ -- 1ms at the
-	 * Boreas default of 1000 Hz, 10ms at the ESP-IDF Kconfig default
-	 * of 100 Hz). Caller-visible semantics match upstream Zephyr's
-	 * wait-queue version (block until expiry or stop, return count
-	 * atomically); the divergence is the polling implementation. */
-	while (__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE) &&
-	       __atomic_load_n(&timer->status, __ATOMIC_ACQUIRE) == 0) {
-		k_msleep(1);
+	/* Upstream shape: return the accumulated count immediately if
+	 * non-zero, or 0 if the timer is stopped; otherwise block until
+	 * expiry or stop. Blocking rides the embedded sync sem, which
+	 * the expiry path and k_timer_stop give (k_sem_give is ISR-safe,
+	 * so this works under both dispatch modes). The sem is a wake
+	 * LATCH, not a counter: a give latched while no waiter was
+	 * blocked makes a later take return early, so re-check and
+	 * re-block in a loop. */
+	for (;;) {
+		uint32_t result = __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
+
+		if (result > 0) {
+			return result;
+		}
+		if (!__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE)) {
+			/* Re-read, don't return 0: an expiry (or a stop after
+			 * an expiry) may have completed between the exchange
+			 * above and the running load. The callback bumps
+			 * status (RELEASE) before clearing running (RELEASE),
+			 * so a running==false ACQUIRE load is guaranteed to
+			 * observe that increment -- upstream returns the
+			 * count here, never drops it. */
+			return __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
+		}
+		(void)k_sem_take(&timer->sync_sem, K_FOREVER);
 	}
-	return __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
 }
 
 uint32_t k_timer_remaining_get(struct k_timer *timer)

components/zkernel/src/k_timer_linux.c

@@ -110,6 +110,8 @@ static void z_timer_dispatcher(void *arg)
 			if (!__atomic_load_n(&due->is_periodic, __ATOMIC_ACQUIRE)) {
 				__atomic_store_n(&due->running, false, __ATOMIC_RELEASE);
 			}
+			/* Wake a status_sync waiter LAST (see k_timer.c). */
+			k_sem_give(&due->sync_sem);
 			continue; /* more timers may already be due */
 		}
 
@@ -164,6 +166,9 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto
 	timer->first_interval_pending = false; /* unused on linux; deadline math is direct */
 	timer->period_us = 0;
 	timer->deadline_us = 0;
+	/* Binary: the sem is the status_sync wake latch, not a counter
+	 * (the expiry count lives in timer->status). */
+	(void)k_sem_init(&timer->sync_sem, 0, 1);
 	timer->node.next = NULL;
 	timer->node.prev = NULL;
 	/* No esp_timer exists on linux; `handle` serves only as the shared
@@ -214,6 +219,9 @@ void k_timer_stop(struct k_timer *timer)
 	if (timer->stop_fn) {
 		timer->stop_fn(timer);
 	}
+	/* Wake a status_sync waiter (upstream unpends one waiter on
+	 * stop, after invoking the stop function). */
+	k_sem_give(&timer->sync_sem);
 }
 
 uint32_t k_timer_status_get(struct k_timer *timer)
@@ -223,13 +231,26 @@ uint32_t k_timer_status_get(struct k_timer *timer)
 
 uint32_t k_timer_status_sync(struct k_timer *timer)
 {
-	/* Same poll-based implementation as the hardware backend in
-	 * k_timer.c -- see the divergence note there. */
-	while (__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE) &&
-	       __atomic_load_n(&timer->status, __ATOMIC_ACQUIRE) == 0) {
-		k_msleep(1);
+	/* Same sem-backed implementation as the hardware backend -- see
+	 * the rationale in k_timer.c. */
+	for (;;) {
+		uint32_t result = __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
+
+		if (result > 0) {
+			return result;
+		}
+		if (!__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE)) {
+			/* Re-read, don't return 0: an expiry (or a stop after
+			 * an expiry) may have completed between the exchange
+			 * above and the running load. The callback bumps
+			 * status (RELEASE) before clearing running (RELEASE),
+			 * so a running==false ACQUIRE load is guaranteed to
+			 * observe that increment -- upstream returns the
+			 * count here, never drops it. */
+			return __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
+		}
+		(void)k_sem_take(&timer->sync_sem, K_FOREVER);
 	}
-	return __atomic_exchange_n(&timer->status, 0, __ATOMIC_ACQ_REL);
 }
 
 /* Locked snapshot of deadline_us: the dispatcher re-arms periodic timers

test/main/test_k_timer.c

@@ -196,6 +196,52 @@ static void test_timer_status_sync_wakes_on_stop(void)
 	k_thread_join(&waker_thread, K_FOREVER);
 }
 
+/* #28 regression: the status_sync wake rides a binary sem LATCH, and a
+ * latched give from an expiry nobody was waiting on must not satisfy a
+ * later status_sync early -- after status_get drains the count, sync
+ * must block until the NEXT expiry (the re-check loop in
+ * k_timer_status_sync is what absorbs the stale latch).
+ *
+ * Deterministic shape (review): one-shot expiries only, and t0 is
+ * captured BEFORE the restart -- the new expiry cannot fire earlier
+ * than start+60, so elapsed >= ~60 holds no matter how the host
+ * stalls the test thread (a periodic re-arm racing the entry was
+ * flaky under CI load). A buggy implementation that honors the stale
+ * latch returns 0 with elapsed ~0. */
+static void test_timer_status_sync_after_status_get_blocks(void)
+{
+	struct k_timer timer;
+	k_timer_init(&timer, test_timer_cb, NULL);
+
+	/* One-shot expires with NO waiter: the latch is left set. */
+	k_timer_start(&timer, K_MSEC(20), K_NO_WAIT);
+	k_msleep(40);
+	TEST_ASSERT_EQUAL(1, k_timer_status_get(&timer));
+
+	/* Restart (start does not drain the latch), then sync. */
+	uint32_t t0 = (uint32_t)k_uptime_get();
+	k_timer_start(&timer, K_MSEC(60), K_NO_WAIT);
+	uint32_t status = k_timer_status_sync(&timer);
+	uint32_t elapsed = (uint32_t)k_uptime_get() - t0;
+
+	TEST_ASSERT_EQUAL(1, status);
+	TEST_ASSERT_GREATER_OR_EQUAL(50, elapsed); /* blocked until the NEW expiry */
+
+	k_timer_stop(&timer);
+}
+
+static void test_timer_status_sync_stopped_returns_immediately(void)
+{
+	struct k_timer timer;
+	k_timer_init(&timer, test_timer_cb, NULL);
+
+	/* Upstream: a stopped/never-started timer with status 0 returns 0
+	 * without blocking. */
+	uint32_t t0 = (uint32_t)k_uptime_get();
+	TEST_ASSERT_EQUAL(0, k_timer_status_sync(&timer));
+	TEST_ASSERT_LESS_THAN(10, (uint32_t)k_uptime_get() - t0);
+}
+
 static void test_timer_remaining_ticks(void)
 {
 	struct k_timer timer;
@@ -455,6 +501,8 @@ void test_k_timer_group(void)
 	RUN_TEST(test_timer_status_sync_blocking);
 	RUN_TEST(test_timer_status_sync_already_fired);
 	RUN_TEST(test_timer_status_sync_wakes_on_stop);
+	RUN_TEST(test_timer_status_sync_after_status_get_blocks);
+	RUN_TEST(test_timer_status_sync_stopped_returns_immediately);
 	RUN_TEST(test_timer_remaining_ticks);
 	RUN_TEST(test_timer_expires_ticks);
 	RUN_TEST(test_timer_one_shot_clears_running);