Skip to content

Commit 6be01e8

Browse files
committed
extmod/zephyr_kernel: Register thread list as GC root pointer.
The static thread list head pointer must be registered via MP_REGISTER_ROOT_POINTER to ensure it is scanned during garbage collection. Without registration, the GC cannot see the pointer and may incorrectly collect the main thread and entire thread list. Changed all references from static 'thread' variable (11 locations) to MP_STATE_VM(mp_thread_list_head) to access the registered root pointer. Use 'struct _mp_thread_t *' in registration (not typedef) for forward declaration compatibility with mpstate.h inclusion order. Signed-off-by: Andrew Leech <[email protected]>
1 parent 1dff6d2 commit 6be01e8

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

extmod/zephyr_kernel/kernel/mpthread_zephyr.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,12 @@ static void check_thread_canaries(mp_thread_t *th, const char *location) {
157157
}
158158
}
159159

160+
// Register thread list head as GC root pointer
161+
// This ensures the main thread and linked list are scanned during GC
162+
MP_REGISTER_ROOT_POINTER(struct _mp_thread_t *mp_thread_list_head);
163+
160164
// Global state
161165
static mp_thread_mutex_t thread_mutex;
162-
static mp_thread_t *thread = NULL; // Thread list head (GC root)
163166
static uint8_t mp_thread_counter;
164167
static mp_thread_stack_slot_t stack_slot[MP_THREAD_MAXIMUM_USER_THREADS];
165168

@@ -214,7 +217,7 @@ bool mp_thread_init(void *stack) {
214217
// Memory barrier to ensure initialization complete
215218
__sync_synchronize();
216219

217-
thread = th; // Set as head of thread list
220+
MP_STATE_VM(mp_thread_list_head) = th; // Set as head of thread list
218221

219222
// Validate canaries after main thread initialization
220223
check_thread_canaries(th, "mp_thread_init main thread");
@@ -231,7 +234,7 @@ void mp_thread_deinit(void) {
231234
mp_thread_mutex_lock(&thread_mutex, 1);
232235

233236
// Abort all threads except current
234-
for (mp_thread_t *th = thread; th != NULL; th = th->next) {
237+
for (mp_thread_t *th = MP_STATE_VM(mp_thread_list_head); th != NULL; th = th->next) {
235238
if ((th->id != k_current_get()) && (th->status != MP_THREAD_STATUS_FINISHED)) {
236239
th->status = MP_THREAD_STATUS_FINISHED;
237240
DEBUG_printf("Aborting thread %s\n", k_thread_name_get(th->id));
@@ -248,7 +251,7 @@ void mp_thread_deinit(void) {
248251
void mp_thread_gc_others(void) {
249252
mp_thread_t *prev = NULL;
250253

251-
if (thread == NULL) {
254+
if (MP_STATE_VM(mp_thread_list_head) == NULL) {
252255
return; // Threading not initialized
253256
}
254257

@@ -259,7 +262,7 @@ void mp_thread_gc_others(void) {
259262
k_thread_foreach(mp_thread_iterate_threads_cb, NULL);
260263

261264
// Clean up finished threads and collect GC roots
262-
mp_thread_t *th = thread;
265+
mp_thread_t *th = MP_STATE_VM(mp_thread_list_head);
263266
while (th != NULL) {
264267
mp_thread_t *next = th->next; // Capture next before any modifications
265268

@@ -271,7 +274,7 @@ void mp_thread_gc_others(void) {
271274
if (prev != NULL) {
272275
prev->next = next;
273276
} else {
274-
thread = next;
277+
MP_STATE_VM(mp_thread_list_head) = next;
275278
}
276279
if (th->slot >= 0 && th->slot < MP_THREAD_MAXIMUM_USER_THREADS) {
277280
stack_slot[th->slot].used = false;
@@ -291,7 +294,7 @@ void mp_thread_gc_others(void) {
291294
DEBUG_printf("GC: Scanning %d threads\n", mp_thread_counter + 1);
292295

293296
// Scan all remaining threads for GC roots
294-
for (mp_thread_t *th = thread; th != NULL; th = th->next) {
297+
for (mp_thread_t *th = MP_STATE_VM(mp_thread_list_head); th != NULL; th = th->next) {
295298
DEBUG_printf("GC: Scanning thread %s\n", k_thread_name_get(th->id));
296299

297300
// Validate canaries during GC scan
@@ -355,7 +358,7 @@ mp_uint_t mp_thread_get_id(void) {
355358
// Mark thread as started (called by new thread)
356359
void mp_thread_start(void) {
357360
mp_thread_mutex_lock(&thread_mutex, 1);
358-
for (mp_thread_t *th = thread; th != NULL; th = th->next) {
361+
for (mp_thread_t *th = MP_STATE_VM(mp_thread_list_head); th != NULL; th = th->next) {
359362
if (th->id == k_current_get()) {
360363
th->status = MP_THREAD_STATUS_READY;
361364
break;
@@ -439,8 +442,8 @@ mp_uint_t mp_thread_create_ex(void *(*entry)(void *), void *arg, size_t *stack_s
439442
th->arg = arg;
440443
th->stack = (void *)th->z_thread.stack_info.start;
441444
th->stack_len = th->z_thread.stack_info.size / sizeof(uintptr_t);
442-
th->next = thread;
443-
thread = th;
445+
th->next = MP_STATE_VM(mp_thread_list_head);
446+
MP_STATE_VM(mp_thread_list_head) = th;
444447

445448
stack_slot[_slot].used = true;
446449
mp_thread_counter++;
@@ -472,7 +475,7 @@ mp_uint_t mp_thread_create(void *(*entry)(void *), void *arg, size_t *stack_size
472475
void mp_thread_finish(void) {
473476
mp_thread_mutex_lock(&thread_mutex, 1);
474477

475-
for (mp_thread_t *th = thread; th != NULL; th = th->next) {
478+
for (mp_thread_t *th = MP_STATE_VM(mp_thread_list_head); th != NULL; th = th->next) {
476479
if (th->id == k_current_get()) {
477480
// Validate canaries before marking thread finished
478481
check_thread_canaries(th, "mp_thread_finish");
@@ -535,7 +538,7 @@ void mp_thread_recursive_mutex_unlock(mp_thread_recursive_mutex_t *mutex) {
535538

536539
// Helper: Thread iteration callback for GC
537540
static void mp_thread_iterate_threads_cb(const struct k_thread *z_thread, void *user_data) {
538-
for (mp_thread_t *th = thread; th != NULL; th = th->next) {
541+
for (mp_thread_t *th = MP_STATE_VM(mp_thread_list_head); th != NULL; th = th->next) {
539542
if (th->id == (struct k_thread *)z_thread) {
540543
th->alive = 1;
541544
DEBUG_printf("GC: Found thread %s\n", k_thread_name_get(th->id));

0 commit comments

Comments
 (0)