Skip to content

Commit 6867c09

Browse files
committed
Fix ISR-safe critical sections and document APIs
The CRITICAL_ENTER/LEAVE macros unconditionally enabled interrupts on leave, which corrupts interrupt state when called from ISR context (where MIE is already 0). Replace with nesting-aware save/restore: CRITICAL_ENTER saves MIE on first entry, CRITICAL_LEAVE restores it on last exit. Add underflow guard to CRITICAL_LEAVE. This fixes mo_logger_enqueue() to use CRITICAL_ENTER instead of mo_mutex_lock so the [ISR-SAFE] contract is actually honored -- mutex blocks, which deadlocks in ISR context. It also fixes realloc() missing CRITICAL_ENTER before fast paths that called CRITICAL_LEAVE, and add defensive CRITICAL_LEAVE before panic() in malloc(). ISR Context Safety Guide is added to hal-interrupt.md with function tables, callback patterns, and ISR-to-task communication examples.
1 parent 91f47ca commit 6867c09

14 files changed

Lines changed: 415 additions & 35 deletions

File tree

Documentation/hal-interrupt.md

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,141 @@ void bad_critical_section(void) {
209209
/* CORRECT */
210210
void good_pattern(void) {
211211
mo_sem_wait(semaphore); /* Block outside critical section */
212-
212+
213213
CRITICAL_ENTER();
214214
/* Quick critical work */
215215
CRITICAL_LEAVE();
216216
}
217217
```
218+
219+
## ISR Context Safety Guide
220+
221+
### Overview
222+
Interrupt Service Routines (ISRs), including timer callbacks, execute in interrupt context with special restrictions.
223+
Violating these restrictions causes heap corruption, deadlocks, or undefined behavior.
224+
225+
### ISR-Safe vs Task-Only Functions
226+
227+
#### ISR-Safe Functions
228+
These functions can be called from ISR context (timer callbacks, trap handlers):
229+
230+
| Function | Protection | Notes |
231+
|----------|------------|-------|
232+
| `mo_task_id()` | None needed | Read-only |
233+
| `mo_task_count()` | None needed | Read-only |
234+
| `mo_ticks()` | None needed | Volatile read |
235+
| `mo_uptime()` | None needed | Calculated |
236+
| `mo_timer_create/destroy/start/cancel()` | NOSCHED | Full timer API |
237+
| `mo_sem_trywait()` | None | Non-blocking |
238+
| `mo_sem_signal()` | NOSCHED | Wakes waiting tasks |
239+
| `mo_mutex_trylock()` | None | Non-blocking |
240+
| `mo_mutex_unlock()` | NOSCHED | Releases lock |
241+
| `mo_cond_signal/broadcast()` | NOSCHED | Wakes waiters |
242+
| `mo_pipe_nbread/nbwrite()` | None | Non-blocking I/O |
243+
| `mo_logger_enqueue()` | CRITICAL | Deferred logging |
244+
| `isr_puts()` | None | Direct UART output |
245+
| `isr_putx()` | None | Direct UART hex output |
246+
247+
#### Task-Only Functions
248+
These functions must NOT be called from ISR context:
249+
250+
| Function | Reason | Alternative |
251+
|----------|--------|-------------|
252+
| `mo_task_spawn()` | Uses malloc | Pre-create tasks |
253+
| `mo_task_cancel()` | Uses free | Defer to task |
254+
| `mo_task_delay/yield()` | Invokes scheduler | Use timer callback |
255+
| `mo_task_suspend/resume()` | Modifies scheduler | Use semaphore |
256+
| `mo_sem_wait()` | Blocks caller | Use `mo_sem_trywait()` |
257+
| `mo_mutex_lock()` | Blocks caller | Use `mo_mutex_trylock()` |
258+
| `mo_cond_wait()` | Blocks caller | Signal from ISR, wait in task |
259+
| `mo_pipe_read/write()` | Blocks caller | Use `mo_pipe_nbread/nbwrite()` |
260+
| `mo_mq_enqueue/dequeue()` | Uses malloc | Use pipe instead |
261+
| `printf()` | May deadlock | Use `mo_logger_enqueue()` |
262+
263+
### Timer Callback Patterns
264+
265+
Timer callbacks execute in ISR context. Example safe callback:
266+
```c
267+
void *my_callback(void *arg) {
268+
/* Safe: ISR-protected logging */
269+
mo_logger_enqueue("Timer fired\n", 12);
270+
271+
/* Safe: Non-blocking semaphore signal to wake task */
272+
mo_sem_signal(signal_sem);
273+
274+
/* Safe: Non-blocking pipe write */
275+
char msg[] = "event";
276+
mo_pipe_nbwrite(event_pipe, msg, sizeof(msg));
277+
278+
/* UNSAFE - do not use in callbacks:
279+
* mo_task_spawn(...); // Uses malloc
280+
* mo_task_delay(10); // Invokes scheduler
281+
* mo_sem_wait(sem); // Blocks caller
282+
* printf(...); // May deadlock
283+
*/
284+
285+
return NULL;
286+
}
287+
```
288+
289+
### ISR-to-Task Communication Patterns
290+
291+
#### Pattern 1: Semaphore Signaling
292+
```c
293+
/* ISR/callback: Signal event */
294+
void *timer_callback(void *arg) {
295+
mo_sem_signal(event_sem);
296+
return NULL;
297+
}
298+
299+
/* Task: Wait for event */
300+
void event_handler_task(void) {
301+
while (1) {
302+
mo_sem_wait(event_sem); /* Blocks until ISR signals */
303+
process_event();
304+
}
305+
}
306+
```
307+
308+
#### Pattern 2: Non-Blocking Pipe
309+
```c
310+
/* ISR/callback: Send data */
311+
void *sensor_callback(void *arg) {
312+
sensor_data_t data = read_sensor();
313+
mo_pipe_nbwrite(sensor_pipe, &data, sizeof(data));
314+
return NULL;
315+
}
316+
317+
/* Task: Process data */
318+
void sensor_task(void) {
319+
sensor_data_t data;
320+
while (1) {
321+
if (mo_pipe_read(sensor_pipe, &data, sizeof(data)) > 0)
322+
process_sensor_data(&data);
323+
}
324+
}
325+
```
326+
327+
#### Pattern 3: Deferred Logging
328+
```c
329+
/* ISR/callback: Queue log message */
330+
void *debug_callback(void *arg) {
331+
mo_logger_enqueue("Tick!\n", 6);
332+
return NULL;
333+
}
334+
/* Logger task automatically outputs queued messages */
335+
```
336+
337+
### Emergency Debug Output
338+
339+
For debugging trap handlers or when the logger is unavailable, use direct UART:
340+
```c
341+
void *debug_isr(void *arg) {
342+
isr_puts("[ISR] Value=0x");
343+
isr_putx(some_value);
344+
isr_puts("\r\n");
345+
return NULL;
346+
}
347+
```
348+
349+
Warning: Direct UART output is blocking and slow. Use only for emergency debugging.

app/timer.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@
22
*
33
* It creates several timers with different periods and modes to verify
44
* their correct operation.
5+
*
6+
* Note: Timer callbacks execute in ISR context. This test uses printf()
7+
* which internally calls mo_logger_enqueue() - an ISR-safe function that
8+
* queues messages for deferred output by the logger task.
59
*/
610
#include <linmo.h>
711

8-
/* Helper function to print the current system uptime. */
9-
void print_time()
12+
/* Helper function to print the current system uptime.
13+
* Called from timer callback (ISR context) - uses ISR-safe logging.
14+
*/
15+
static void print_time(void)
1016
{
1117
/* mo_uptime() returns a 64-bit value to prevent rollover. */
1218
uint64_t time_ms = mo_uptime();
@@ -17,8 +23,9 @@ void print_time()
1723
}
1824

1925
/* Generic timer callback.
20-
* We can use a single callback for multiple timers by passing a unique
21-
* identifier as the argument.
26+
* Executes in ISR context - only ISR-safe functions are permitted here.
27+
* printf() is safe because it uses mo_logger_enqueue() internally.
28+
* Alternative ISR-safe options: mo_logger_enqueue(), isr_puts(), isr_putx()
2229
*/
2330
void *timer_callback(void *arg)
2431
{

include/lib/libc.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,42 @@ int random_r(struct random_data *buf, int32_t *result);
143143
int32_t puts(const char *str);
144144
int _putchar(int c); /* Low-level character output (used by logger) */
145145

146+
/* ISR-Safe I/O Functions
147+
* [ISR-SAFE] These functions are safe to call from interrupt context.
148+
* They perform direct UART output without locks, buffering, or malloc.
149+
* Use for emergency debug output in timer callbacks and trap handlers.
150+
*
151+
* Warning: Direct UART output is slow and blocks until complete.
152+
* For normal logging, prefer mo_logger_enqueue() which is non-blocking.
153+
*/
154+
155+
/* ISR-safe string output - direct UART, no buffering.
156+
* [ISR-SAFE] Safe to call from any context including ISR and timer callbacks.
157+
*
158+
* @s : Null-terminated string to output
159+
*/
160+
static inline void isr_puts(const char *s)
161+
{
162+
while (*s)
163+
_putchar(*s++);
164+
}
165+
166+
/* ISR-safe hexadecimal output - direct UART, no buffering.
167+
* [ISR-SAFE] Safe to call from any context including ISR and timer callbacks.
168+
*
169+
* Outputs a 32-bit value as 8 hex digits (e.g., "DEADBEEF").
170+
* Useful for printing addresses and error codes in trap handlers.
171+
*
172+
* @val : 32-bit value to output as hexadecimal
173+
*/
174+
static inline void isr_putx(uint32_t val)
175+
{
176+
for (int i = 28; i >= 0; i -= 4) {
177+
uint32_t nibble = (val >> i) & 0xF;
178+
_putchar(nibble < 10 ? '0' + nibble : 'A' + nibble - 10);
179+
}
180+
}
181+
146182
/* Character and string input */
147183
int getchar(void);
148184
char *gets(char *s);

include/linmo.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,52 @@
11
#pragma once
22

3+
/* Linmo Operating System - Main API Header
4+
*
5+
* This header includes all kernel APIs for task management, synchronization,
6+
* IPC, and system services.
7+
*
8+
* Interrupt Service Routines (ISRs), including timer callbacks, execute in
9+
* interrupt context with special restrictions. Violating these restrictions
10+
* causes heap corruption, deadlocks, or undefined behavior.
11+
*
12+
* [ISR-SAFE] Functions (callable from interrupt context):
13+
* -------------------------------------------------------
14+
* System Info: mo_task_id(), mo_task_count(), mo_ticks(), mo_uptime()
15+
* Timer API: mo_timer_create/destroy/start/cancel() [NOSCHED protected]
16+
* Semaphore: mo_sem_trywait(), mo_sem_signal() [non-blocking]
17+
* Mutex: mo_mutex_trylock(), mo_mutex_unlock() [non-blocking]
18+
* Condition Var: mo_cond_signal(), mo_cond_broadcast() [non-blocking]
19+
* Pipe I/O: mo_pipe_nbread(), mo_pipe_nbwrite() [non-blocking]
20+
* Logging: mo_logger_enqueue() [CRITICAL protected]
21+
* Direct I/O: _putchar() for emergency/debug output
22+
*
23+
* [TASK-ONLY] Functions (must NOT call from ISR context):
24+
* -------------------------------------------------------
25+
* Memory: mo_task_spawn(), mo_task_cancel() - use malloc/free
26+
* Blocking: mo_task_delay(), mo_task_yield(), mo_task_suspend()
27+
* Blocking Sync: mo_sem_wait(), mo_mutex_lock(), mo_cond_wait()
28+
* Blocking I/O: mo_pipe_read(), mo_pipe_write()
29+
* Message Queue: mo_mq_enqueue(), mo_mq_dequeue() - unprotected malloc
30+
* Stdio: printf(), puts() - may deadlock in preemptive mode
31+
*
32+
* Timer Callback Rules:
33+
* ---------------------
34+
* Timer callbacks execute in ISR context. Example safe callback:
35+
*
36+
* void *my_callback(void *arg) {
37+
* mo_logger_enqueue("Timer fired\n", 12); // Safe: ISR-protected
38+
* mo_sem_signal(signal_sem); // Safe: non-blocking
39+
* // UNSAFE: mo_task_spawn(...); // Uses malloc
40+
* // UNSAFE: printf(...); // May deadlock
41+
* return NULL;
42+
* }
43+
*
44+
* For emergency debug output in ISR, use the ISR-safe I/O functions:
45+
* isr_puts("message"); // Direct UART string output
46+
* isr_putx(0xDEADBEEF); // Direct UART hex output
47+
* These are defined in lib/libc.h and available via linmo.h.
48+
*/
49+
350
#include <types.h>
451

552
#include <lib/libc.h>

include/sys/logger.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,15 @@
3737
int32_t mo_logger_init(void);
3838

3939
/* Enqueue a log message for deferred output.
40+
* [ISR-SAFE] Protected by CRITICAL_ENTER - safe to call from ISR context
41+
*
4042
* Non-blocking: if queue is full, message is dropped.
4143
* Thread-safe: protected by short critical section.
44+
*
45+
* This is the RECOMMENDED way to log from timer callbacks and ISRs.
46+
* The message is queued and output later by the logger task, avoiding
47+
* the risk of deadlock from direct UART output in ISR context.
48+
*
4249
* @msg : Null-terminated message string
4350
* @length : Length of message (excluding null terminator)
4451
*

include/sys/mqueue.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@
77
* Provides FIFO message passing between tasks with type-safe message
88
* containers. Messages consist of a user-allocated payload, a type
99
* discriminator, and size information.
10+
*
11+
* ISR-SAFETY WARNING:
12+
* [TASK-ONLY] All message queue operations are task-context only.
13+
* Message queue operations use underlying queue functions that may perform
14+
* memory allocation. These operations are NOT protected by CRITICAL_ENTER
15+
* and are NOT safe to call from ISR context.
16+
*
17+
* For ISR-to-task communication, use:
18+
* - mo_pipe_nbwrite() for simple byte streams
19+
* - mo_sem_signal() to wake a task
20+
* - mo_logger_enqueue() for logging
1021
*/
1122

1223
/* Message container structure */

include/sys/mutex.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ int32_t mo_mutex_destroy(mutex_t *m);
5454
/* Mutex Locking Operations */
5555

5656
/* Acquire mutex lock, blocking if necessary.
57+
* [TASK-ONLY] May block caller - NOT safe from ISR context
58+
*
5759
* If the mutex is free, acquires it immediately. If owned by another task,
5860
* the calling task is placed in the wait queue and blocked until the mutex
5961
* becomes available. Non-recursive - returns error if caller already owns
@@ -65,6 +67,8 @@ int32_t mo_mutex_destroy(mutex_t *m);
6567
int32_t mo_mutex_lock(mutex_t *m);
6668

6769
/* Attempt to acquire mutex lock without blocking.
70+
* [ISR-SAFE] Non-blocking, returns immediately with success/failure
71+
*
6872
* Returns immediately whether the lock was acquired or not.
6973
* @m : Pointer to mutex structure (must be valid)
7074
*
@@ -73,6 +77,8 @@ int32_t mo_mutex_lock(mutex_t *m);
7377
int32_t mo_mutex_trylock(mutex_t *m);
7478

7579
/* Attempt to acquire mutex lock with timeout.
80+
* [TASK-ONLY] May block caller - NOT safe from ISR context
81+
*
7682
* Blocks for up to 'ticks' scheduler ticks waiting for the mutex.
7783
* @m : Pointer to mutex structure (must be valid)
7884
* @ticks : Maximum time to wait in scheduler ticks (0 = trylock behavior)
@@ -83,6 +89,8 @@ int32_t mo_mutex_trylock(mutex_t *m);
8389
int32_t mo_mutex_timedlock(mutex_t *m, uint32_t ticks);
8490

8591
/* Release mutex lock.
92+
* [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context
93+
*
8694
* If tasks are waiting, ownership is transferred to the next task in FIFO
8795
* order. The released task is marked ready but may not run immediately
8896
* depending on scheduler priority.
@@ -142,6 +150,8 @@ int32_t mo_cond_destroy(cond_t *c);
142150
/* Condition Variable Wait Operations */
143151

144152
/* Wait on condition variable (atomically releases mutex).
153+
* [TASK-ONLY] Blocks caller - NOT safe from ISR context
154+
*
145155
* The calling task must own the specified mutex. The mutex is atomically
146156
* released and the task blocks until another task signals the condition.
147157
* Upon waking, the mutex is re-acquired before returning.
@@ -153,6 +163,8 @@ int32_t mo_cond_destroy(cond_t *c);
153163
int32_t mo_cond_wait(cond_t *c, mutex_t *m);
154164

155165
/* Wait on condition variable with timeout.
166+
* [TASK-ONLY] Blocks caller - NOT safe from ISR context
167+
*
156168
* Like mo_cond_wait(), but limits wait time to specified number of ticks.
157169
* @c : Pointer to condition variable structure (must be valid)
158170
* @m : Pointer to mutex structure that caller must own
@@ -166,6 +178,8 @@ int32_t mo_cond_timedwait(cond_t *c, mutex_t *m, uint32_t ticks);
166178
/* Condition Variable Signal Operations */
167179

168180
/* Signal one waiting task.
181+
* [ISR-SAFE] Protected by NOSCHED_ENTER - safe for ISR-to-task signaling
182+
*
169183
* Wakes up the oldest task waiting on the condition variable (FIFO order).
170184
* The signaled task will attempt to re-acquire the associated mutex.
171185
* @c : Pointer to condition variable structure (must be valid)
@@ -175,6 +189,8 @@ int32_t mo_cond_timedwait(cond_t *c, mutex_t *m, uint32_t ticks);
175189
int32_t mo_cond_signal(cond_t *c);
176190

177191
/* Signal all waiting tasks.
192+
* [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context
193+
*
178194
* Wakes up all tasks waiting on the condition variable. All tasks will
179195
* attempt to re-acquire their associated mutexes, but only one will succeed
180196
* immediately (others will block on the mutex).

0 commit comments

Comments
 (0)