[kernel] Prevent duplicate defunct enqueue for deleted threads.

.github/workflows/utest_auto_run.yml

@@ -392,9 +392,12 @@ jobs:
                 -append "console=ttyS0 earlycon=sbi" \
                 > qemu_output_$TEST_QEMU_ARCH.log 2>&1 &
           elif [[ "${{ matrix.platform.QEMU_ARCH }}"  == "arm" ]]; then 
+            cpus=$(awk '/^#define[[:space:]]+RT_CPUS_NR[[:space:]]/ { print $3 }' "$TEST_BSP_ROOT/rtconfig.h")
+            cpus=${cpus:-1}
             qemu-system-$TEST_QEMU_ARCH \
                 -nographic \
                 -M $TEST_QEMU_MACHINE \
+                -smp "$cpus" \
                 -kernel $TEST_BSP_ROOT/rtthread.bin  \
                 -sd $TEST_BSP_ROOT/sd.bin  \
                 > qemu_output_$TEST_QEMU_ARCH.log  2>&1 &
@@ -432,9 +435,9 @@ jobs:
           echo "=========================================================================================="
           set +e
           timeout 20m bash -c '
-            tail -n +1 -f "$1" | while IFS= read -r line; do
+            while IFS= read -r line; do
               echo "$line"
-              if [[ "$line" == *"[  FAILED  ] [ result   ]"* ]]; then
+              if [[ "$line" == *"[  FAILED  ] [ result   ]"* || "$line" == *"[  ASSERT  ] [ unit     ]"* ]]; then
                 ERROR_LOGS="${ERROR_LOGS}${line}"$'\''\n'\''
                 FAILURE_DETECTED=true
               fi
@@ -463,7 +466,7 @@ jobs:
                 echo "=========================================================================================="
                 exit 0
               fi
-            done
+            done < <(tail --pid=$$ -n +1 -f "$1")
           ' bash "$LOG_FILE"
           monitor_status=$?
           set -e

components/drivers/ipc/utest/workqueue_tc.c

@@ -334,11 +334,19 @@ static void repeat_work_test01_fun(struct rt_work *work, void *work_data)
     *((int *)work_data) += 1;
 }
 
+static void hold_workqueue(struct rt_work *work, void *work_data)
+{
+    RT_UNUSED(work);
+    rt_completion_wait((struct rt_completion *)work_data, RT_WAITING_FOREVER);
+}
+
 static void repeat_work_test01(void)
 {
     struct rt_workqueue *queue;
     rt_uint8_t curr_priority;
     struct rt_work work;
+    struct rt_work blocker;
+    struct rt_completion release;
     volatile int work_flag = 0;
     rt_err_t err;
 
@@ -351,6 +359,11 @@ static void repeat_work_test01(void)
         return;
     }
     work_flag = 0;
+    /* Keep the target pending until both submissions finish, including on SMP. */
+    rt_completion_init(&release);
+    rt_work_init(&blocker, hold_workqueue, &release);
+    err = rt_workqueue_submit_work(queue, &blocker, 0);
+    uassert_int_equal(err, RT_EOK);
     rt_work_init(&work, repeat_work_test01_fun, (void *)&work_flag);
     /* Multiple submissions of the same work */
     err = rt_workqueue_submit_work(queue, &work, 0);
@@ -362,6 +375,7 @@ static void repeat_work_test01(void)
     {
         LOG_E("L:%d err. %d", __LINE__, err);
     }
+    rt_completion_done(&release);
     rt_thread_delay(100);
     /* Check if it was executed only once */
     uassert_int_equal(work_flag, 1);
@@ -415,22 +429,32 @@ static void repeat_work_test02(void)
 
 static struct rt_workqueue *queue_3;
 
+struct repeat_work_context
+{
+    int count;
+    struct rt_completion done;
+};
+
 static void repeat_work_test03_fun(struct rt_work *work, void *work_data)
 {
-    int *work_flag = (int *)work_data;
-    (*work_flag) += 1;
-    rt_kprintf("work_flag:%d\n", *work_flag);
-    if (*work_flag < 20)
+    struct repeat_work_context *context = work_data;
+    context->count += 1;
+    rt_kprintf("work_flag:%d\n", context->count);
+    if (context->count < 20)
     {
         rt_workqueue_submit_work(queue_3, work, 0);
     }
+    else
+    {
+        rt_completion_done(&context->done);
+    }
 }
 
 static void repeat_work_test03(void)
 {
     rt_uint8_t curr_priority;
     struct rt_work work;
-    volatile int work_flag = 0;
+    struct repeat_work_context context;
     rt_err_t err;
 
     /* 1 priority higher than current test thread */
@@ -441,17 +465,21 @@ static void repeat_work_test03(void)
         LOG_E("queue create failed, L:%d", __LINE__);
         return;
     }
-    rt_work_init(&work, repeat_work_test03_fun, (void *)&work_flag);
+    context.count = 0;
+    rt_completion_init(&context.done);
+    rt_work_init(&work, repeat_work_test03_fun, &context);
     /* Submit work with high queue priority that will be executed immediately */
     err = rt_workqueue_submit_work(queue_3, &work, 0);
     uassert_int_equal(err, RT_EOK);
 
     /* Wait for the work to be executed 20 times with a timeout */
+    err = rt_completion_wait(&context.done, rt_tick_from_millisecond(5000));
+    uassert_int_equal(err, RT_EOK);
     err = rt_workqueue_cancel_work_sync(queue_3, &work);
     uassert_int_equal(err, RT_EOK);
 
     /* Check if the work was executed 20 times */
-    uassert_int_equal(work_flag, 20);
+    uassert_int_equal(context.count, 20);
 
     rt_workqueue_destroy(queue_3);
 }

include/rtdef.h

@@ -957,6 +957,7 @@ struct rt_thread
 #endif /* RT_USING_MEM_PROTECTION */
 
     struct rt_spinlock          spinlock;
+    rt_uint8_t defunct;                                 /**< cleanup claimed, protected by spinlock */
     rt_ubase_t                  user_data;              /**< private user data beyond this thread */
 };
 typedef struct rt_thread *rt_thread_t;

src/thread.c

@@ -116,6 +116,26 @@ static void _thread_detach_from_mutex(rt_thread_t thread)
 }
 #endif
 
+static rt_err_t _thread_mark_defunct(rt_thread_t thread)
+{
+    rt_err_t error = -RT_ERROR;
+    rt_base_t level;
+
+    /*
+     * Scheduler bit-fields use different locking, so this claim has separate
+     * storage. Only thread initialization clears it for a new lifetime.
+     */
+    level = rt_spin_lock_irqsave(&thread->spinlock);
+    if (!thread->defunct)
+    {
+        thread->defunct = RT_TRUE;
+        error = RT_EOK;
+    }
+    rt_spin_unlock_irqrestore(&thread->spinlock, level);
+
+    return error;
+}
+
 static void _thread_exit(void)
 {
     struct rt_thread *thread;
@@ -128,10 +148,13 @@ static void _thread_exit(void)
 
     rt_thread_close(thread);
 
-    _thread_detach_from_mutex(thread);
+    if (_thread_mark_defunct(thread) == RT_EOK)
+    {
+        _thread_detach_from_mutex(thread);
 
-    /* insert to defunct thread list */
-    rt_thread_defunct_enqueue(thread);
+        /* insert to defunct thread list */
+        rt_thread_defunct_enqueue(thread);
+    }
 
     rt_exit_critical_safe(critical_level);
 
@@ -233,6 +256,7 @@ static rt_err_t _thread_init(struct rt_thread *thread,
 
     /* error and flags */
     thread->error = RT_EOK;
+    thread->defunct = RT_FALSE;
 
     /* lock init */
 #ifdef RT_USING_SMP
@@ -518,10 +542,17 @@ static rt_err_t _thread_detach(rt_thread_t thread)
 
     error = rt_thread_close(thread);
 
-    _thread_detach_from_mutex(thread);
+    if (error == RT_EOK)
+    {
+        error = _thread_mark_defunct(thread);
+        if (error == RT_EOK)
+        {
+            _thread_detach_from_mutex(thread);
 
-    /* insert to defunct thread list */
-    rt_thread_defunct_enqueue(thread);
+            /* insert to defunct thread list */
+            rt_thread_defunct_enqueue(thread);
+        }
+    }
 
     rt_exit_critical_safe(critical_level);
     return error;

src/utest/thread_suspend_tc.c

@@ -16,6 +16,23 @@
 #define THREAD_TIMESLICE     5
 #define TEST_THREAD_PRIORITY 25
 
+#ifdef RT_USING_SMP
+static rt_ubase_t saved_bind_cpu;
+#endif
+
+static rt_err_t start_test_thread(rt_thread_t thread)
+{
+#ifdef RT_USING_SMP
+    /* Forced suspend cannot target a thread running on another CPU. */
+    rt_err_t result = rt_thread_control(thread, RT_THREAD_CTRL_BIND_CPU, (void *)0);
+    if (result != RT_EOK)
+    {
+        return result;
+    }
+#endif
+    return rt_thread_startup(thread);
+}
+
 /* Global variables for normal usage test */
 static rt_thread_t target_thread = RT_NULL;
 static rt_thread_t monitor_thread = RT_NULL;
@@ -203,8 +220,8 @@ static void test_suspend_force_normal_usage(void)
     uassert_not_null(monitor_thread);
 
     /* Start threads */
-    rt_thread_startup(target_thread);
-    rt_thread_startup(monitor_thread);
+    uassert_int_equal(start_test_thread(target_thread), RT_EOK);
+    uassert_int_equal(start_test_thread(monitor_thread), RT_EOK);
 
     /* Wait for test completion */
     rt_sem_take(sync_sem, RT_WAITING_FOREVER);
@@ -257,7 +274,7 @@ static void test_suspend_force_api_basic(void)
 
     uassert_not_null(api_thread);
 
-    rt_thread_startup(api_thread);
+    uassert_int_equal(start_test_thread(api_thread), RT_EOK);
     rt_thread_mdelay(50); /* Wait for thread to start */
 
     uassert_true(thread_started);
@@ -315,7 +332,7 @@ static void test_suspend_force_not_started_thread(void)
     uassert_true(resume_result == -RT_EINVAL);
 
     /* Now start the thread to see if it works normally */
-    rt_err_t startup_result = rt_thread_startup(not_started_thread);
+    rt_err_t startup_result = start_test_thread(not_started_thread);
     uassert_true(startup_result == RT_EOK);
 
     /* Wait a bit to see if thread starts normally */
@@ -352,14 +369,14 @@ static void test_suspend_force_deadlock_risk(void)
     holder_thread = rt_thread_create("holder", mutex_holder_thread, RT_NULL,
                                      THREAD_STACK_SIZE, UTEST_THR_PRIORITY, THREAD_TIMESLICE);
     uassert_not_null(holder_thread);
-    rt_thread_startup(holder_thread);
+    uassert_int_equal(start_test_thread(holder_thread), RT_EOK);
 
     /* Create and start waiter thread */
     waiter_thread = rt_thread_create("waiter", mutex_waiter_thread, RT_NULL,
                                      THREAD_STACK_SIZE, UTEST_THR_PRIORITY + 1, THREAD_TIMESLICE);
     uassert_not_null(waiter_thread);
     /* Now start waiter thread, it will try to acquire mutex held by suspended thread */
-    rt_thread_startup(waiter_thread);
+    uassert_int_equal(start_test_thread(waiter_thread), RT_EOK);
 
     /* Wait for holder to get mutex */
     int timeout = 100; /* 1 second timeout */
@@ -425,7 +442,18 @@ static void test_suspend_force_deadlock_risk(void)
 
 static rt_err_t utest_tc_init(void)
 {
+#ifdef RT_USING_SMP
+    rt_thread_t self = rt_thread_self();
+    rt_sched_lock_level_t level;
+
+    rt_sched_lock(&level);
+    saved_bind_cpu = RT_SCHED_CTX(self).bind_cpu;
+    rt_sched_unlock(level);
+    /* Keep the controller and its targets on the same CPU. */
+    return rt_thread_control(self, RT_THREAD_CTRL_BIND_CPU, (void *)0);
+#else
     return RT_EOK;
+#endif
 }
 
 static rt_err_t utest_tc_cleanup(void)
@@ -464,7 +492,11 @@ static rt_err_t utest_tc_cleanup(void)
     /* Give system time to complete cleanup */
     rt_thread_mdelay(50);
 
+#ifdef RT_USING_SMP
+    return rt_thread_control(rt_thread_self(), RT_THREAD_CTRL_BIND_CPU, (void *)saved_bind_cpu);
+#else
     return RT_EOK;
+#endif
 }
 
 static void testcase(void)