Fix ISR-safe critical sections and document APIs #72
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| return (void *) (old_block + 1); | ||
| } | ||
|
|
||
| CRITICAL_LEAVE(); | ||
|
|
||
| void *new_buf = malloc(size); | ||
| if (new_buf) { | ||
| memcpy(new_buf, ptr, min(old_size, size)); | ||
| free(ptr); | ||
| } | ||
|
|
||
| return new_buf; |
There was a problem hiding this comment.
The realloc function drops the critical section before calling malloc(size), opening a race window. For safety, realloc should typically hold the lock throughout the operation, or strictly enforce that shared pointers must be protected by external locks.
| */ | ||
| volatile uint32_t critical_nesting = 0; | ||
| volatile int32_t critical_saved_mie = 0; | ||
|
|
There was a problem hiding this comment.
I noticed that critical_nesting and critical_saved_mie are implemented as global variables. Is it intended for Single Core? If so, it would be better to store them in TCB, or at least document this limitation in the file. Otherwise, it is not thread-safe. Any future change allowing preemption or yielding within a critical section would corrupt the interrupt state.
There was a problem hiding this comment.
Is it intended for Single Core? If so, it would be better to store them in TCB, or at least document this limitation in the file.
The comments were refined to address SMP awareness.
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.
Linmo CI Test ResultsOverall Status: ✅ passed Toolchain Results
Application Tests
Functional Test Details
Report generated from |
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.
Summary by cubic
Make critical sections ISR-safe by saving/restoring interrupt state with nesting. Prevents interrupt-state corruption and ISR deadlocks; updates logger/allocator and adds clear ISR safety guidance.
Bug Fixes
New Features
Written for commit 3474abb. Summary will update on new commits.