Skip to content

Commit f314457

Browse files
committed
smp: ensure safe thread termination in rt_thread_close
1 parent f9564d4 commit f314457

File tree

1 file changed

+90
-9
lines changed

1 file changed

+90
-9
lines changed

src/thread.c

Lines changed: 90 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2006-2022, RT-Thread Development Team
2+
* Copyright (c) 2006-2025, RT-Thread Development Team
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*
@@ -116,6 +116,7 @@ static void _thread_detach_from_mutex(rt_thread_t thread) {}
116116

117117
static void _thread_exit(void)
118118
{
119+
rt_err_t error;
119120
struct rt_thread *thread;
120121
rt_base_t critical_level;
121122

@@ -124,12 +125,19 @@ static void _thread_exit(void)
124125

125126
critical_level = rt_enter_critical();
126127

127-
rt_thread_close(thread);
128+
error=rt_thread_close(thread);
128129

129-
_thread_detach_from_mutex(thread);
130+
/**
131+
* this check prevents concurrent _thread_detach operations by other threads.
132+
* proceeds only when the current thread has successfully closed the target thread.
133+
*/
134+
if(error == RT_EOK)
135+
{
136+
_thread_detach_from_mutex(thread);
130137

131-
/* insert to defunct thread list */
132-
rt_thread_defunct_enqueue(thread);
138+
/* insert to defunct thread list */
139+
rt_thread_defunct_enqueue(thread);
140+
}
133141

134142
rt_exit_critical_safe(critical_level);
135143

@@ -423,12 +431,15 @@ RTM_EXPORT(rt_thread_startup);
423431
*/
424432
rt_err_t rt_thread_close(rt_thread_t thread)
425433
{
434+
rt_err_t error;
426435
rt_sched_lock_level_t slvl;
427436
rt_uint8_t thread_status;
428437

429438
/* forbid scheduling on current core if closing current thread */
430439
RT_ASSERT(thread != rt_thread_self() || rt_critical_level());
431440

441+
error = RT_EOK;
442+
432443
/* before checking status of scheduler */
433444
rt_sched_lock(&slvl);
434445

@@ -447,12 +458,73 @@ rt_err_t rt_thread_close(rt_thread_t thread)
447458

448459
/* change stat */
449460
rt_sched_thread_close(thread);
461+
462+
#ifdef RT_USING_SMP
463+
int cpu_id;
464+
rt_tick_t start_tick = rt_tick_get();
465+
/**
466+
* using conservative 500ms timeout, may adjust based on
467+
* hardware characteristics and system load.
468+
*/
469+
rt_tick_t timeout = RT_TICK_PER_SECOND >> 1;
470+
rt_bool_t need_wait = RT_FALSE;
471+
472+
/**
473+
* in SMP, the current thread and target thread may run on different CPUs.
474+
* although we set the target thread's state to closed, it may still execute
475+
* on another CPU until rescheduled. send IPI to force immediate rescheduling.
476+
*/
477+
cpu_id = RT_SCHED_CTX(thread).oncpu;
478+
rt_sched_unlock(slvl);
479+
if ((cpu_id != RT_CPU_DETACHED) && (cpu_id != rt_cpu_get_id()))
480+
{
481+
rt_hw_ipi_send(RT_SCHEDULE_IPI, RT_CPU_MASK ^ (1 << cpu_id));
482+
need_wait = RT_TRUE;
483+
}
484+
485+
/**
486+
* continuously check if target thread has detached from CPU core.
487+
* this loop ensures the thread fully stops before resource cleanup.
488+
* a timeout prevents deadlock if thread fails to detach promptly.
489+
*/
490+
while (need_wait)
491+
{
492+
if (rt_tick_get_delta(start_tick) >= timeout)
493+
{
494+
LOG_D("Timeout waiting for thread %s (tid=%p) to detach from CPU%d",
495+
RT_NAME_MAX, thread->parent.name, thread, cpu_id);
496+
error = -RT_ETIMEOUT;
497+
break;
498+
}
499+
500+
rt_hw_cpu_relax();
501+
502+
rt_sched_lock(&slvl);
503+
cpu_id = RT_SCHED_CTX(thread).oncpu;
504+
rt_sched_unlock(slvl);
505+
506+
if (cpu_id == RT_CPU_DETACHED)
507+
{
508+
break;
509+
}
510+
}
511+
512+
return error;
513+
#endif
514+
}
515+
else
516+
{
517+
/**
518+
* avoid duplicate closing: if the thread is already closed, return an error.
519+
* this prevents race conditions when multiple threads call _thread_detach/_thread_exit concurrently.
520+
*/
521+
error = -RT_ERROR;
450522
}
451523

452524
/* scheduler works are done */
453525
rt_sched_unlock(slvl);
454526

455-
return RT_EOK;
527+
return error;
456528
}
457529
RTM_EXPORT(rt_thread_close);
458530

@@ -491,10 +563,18 @@ static rt_err_t _thread_detach(rt_thread_t thread)
491563

492564
error = rt_thread_close(thread);
493565

494-
_thread_detach_from_mutex(thread);
566+
/**
567+
* prevents concurrent thread cleanup operations by synchronizing between external
568+
* _thread_detach calls and the target thread's own _thread_exit path.
569+
* proceeds only when the current thread has successfully closed the target thread.
570+
*/
571+
if (error == RT_EOK)
572+
{
573+
_thread_detach_from_mutex(thread);
495574

496-
/* insert to defunct thread list */
497-
rt_thread_defunct_enqueue(thread);
575+
/* insert to defunct thread list */
576+
rt_thread_defunct_enqueue(thread);
577+
}
498578

499579
rt_exit_critical_safe(critical_level);
500580
return error;
@@ -1143,3 +1223,4 @@ rt_err_t rt_thread_get_name(rt_thread_t thread, char *name, rt_uint8_t name_size
11431223
RTM_EXPORT(rt_thread_get_name);
11441224

11451225
/**@}*/
1226+

0 commit comments

Comments
 (0)