mirror of
https://github.com/eclipse-threadx/threadx.git
synced 2026-10-06 06:59:08 +08:00
Fixed the kernel object leaks on the static creation error paths (#584)
xQueueCreateStatic() and xTaskCreateStatic() take their storage from the caller, so neither leaks memory, but both create ThreadX objects and both return NULL when a later step fails. The caller is left without a handle and cannot call the matching delete function, so any object already created stays registered in the kernel, pointing into a caller buffer that the application is now free to reuse or discard. Three paths were affected. xQueueCreateStatic() abandoned the read semaphore when the write semaphore could not be created. xTaskCreateStatic() abandoned the notification semaphore when the thread could not be created, and abandoned both the semaphore and the thread when the thread could not be resumed. Delete what was already created before returning on each of them. The resume path terminates the thread before deleting it, since a thread created with TX_DONT_START is suspended rather than terminated, which is the same order the idle task uses when it reaps a deleted task. Extend the regression suite to cover all three paths, and add thread resume to the set of entry points the harness can force to fail. Each static failure case now uses its own control block, so a future regression on one path cannot carry damage into the next case and report misleading counts there. Verified against the layer as it stands on dev, where the three new checks fail with the objects left behind, and against the fixed layer, where the suite passes. Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
This commit is contained in:
@@ -45,7 +45,8 @@ set(wrapped_symbols
|
||||
_txe_timer_create
|
||||
_txe_timer_delete
|
||||
_txe_thread_create
|
||||
_txe_thread_delete)
|
||||
_txe_thread_delete
|
||||
_txe_thread_resume)
|
||||
|
||||
set(wrap_link_options "")
|
||||
foreach(symbol ${wrapped_symbols})
|
||||
|
||||
@@ -96,6 +96,22 @@ void txfr_test_body(void)
|
||||
txfr_test_check_counts("vQueueDelete on static queue", &counters, 0, 0, 0, 2);
|
||||
}
|
||||
|
||||
/* The static variant owns no memory, but it still creates two semaphores
|
||||
and still hands back nothing on failure, so the caller cannot call
|
||||
vQueueDelete(). A read semaphore left behind here would stay registered
|
||||
in the kernel, pointing into the caller's StaticQueue_t. */
|
||||
txfr_test_account_start(TXFR_INJECT_SEMAPHORE_CREATE, 1);
|
||||
queue = xQueueCreateStatic(QUEUE_LENGTH, QUEUE_ITEM_SIZE, queue_storage, &queue_control_block);
|
||||
txfr_test_account_stop(&counters);
|
||||
txfr_test_check("xQueueCreateStatic returns NULL on read_sem failure", queue == NULL);
|
||||
txfr_test_check_counts("static read_sem failure leaves nothing behind", &counters, 0, 0, 1, 0);
|
||||
|
||||
txfr_test_account_start(TXFR_INJECT_SEMAPHORE_CREATE, 2);
|
||||
queue = xQueueCreateStatic(QUEUE_LENGTH, QUEUE_ITEM_SIZE, queue_storage, &queue_control_block);
|
||||
txfr_test_account_stop(&counters);
|
||||
txfr_test_check("xQueueCreateStatic returns NULL on write_sem failure", queue == NULL);
|
||||
txfr_test_check_counts("static write_sem failure unwinds read_sem", &counters, 0, 0, 2, 1);
|
||||
|
||||
/* Repeated create and delete cycles must leave the pool able to serve the
|
||||
same request. This would not catch a small leak on its own, which is why
|
||||
the counts above are checked directly, but it does catch a descriptor
|
||||
|
||||
@@ -36,6 +36,18 @@
|
||||
static StaticTask_t task_control_block;
|
||||
static StackType_t task_stack[TASK_STACK_DEPTH];
|
||||
|
||||
/* Each failure case gets its own buffers, for two reasons. vTaskDelete() only
|
||||
queues a task for the idle task to reap, so the control block of a task
|
||||
deleted above is still live while this test thread runs. And if one of these
|
||||
paths ever regresses and leaves an object behind, a shared buffer would
|
||||
carry that damage into the next case and report misleading counts there. */
|
||||
static StaticTask_t semaphore_failure_control_block;
|
||||
static StackType_t semaphore_failure_stack[TASK_STACK_DEPTH];
|
||||
static StaticTask_t thread_failure_control_block;
|
||||
static StackType_t thread_failure_stack[TASK_STACK_DEPTH];
|
||||
static StaticTask_t resume_failure_control_block;
|
||||
static StackType_t resume_failure_stack[TASK_STACK_DEPTH];
|
||||
|
||||
|
||||
static void test_task_entry(void *p_arg)
|
||||
{
|
||||
@@ -102,4 +114,32 @@ void txfr_test_body(void)
|
||||
{
|
||||
vTaskDelete(task);
|
||||
}
|
||||
|
||||
/* As with the queues, the static variant owns no memory but still creates
|
||||
kernel objects and still returns nothing on failure. A semaphore or a
|
||||
thread left behind here stays registered in the kernel, pointing into
|
||||
the caller's StaticTask_t. */
|
||||
txfr_test_account_start(TXFR_INJECT_SEMAPHORE_CREATE, 1);
|
||||
task = xTaskCreateStatic(test_task_entry, "s_sem", TASK_STACK_DEPTH, NULL, TASK_PRIORITY,
|
||||
semaphore_failure_stack, &semaphore_failure_control_block);
|
||||
txfr_test_account_stop(&counters);
|
||||
txfr_test_check("xTaskCreateStatic returns NULL on semaphore failure", task == NULL);
|
||||
txfr_test_check_counts("static semaphore failure leaves nothing behind", &counters, 0, 0, 1, 0);
|
||||
|
||||
txfr_test_account_start(TXFR_INJECT_THREAD_CREATE, 1);
|
||||
task = xTaskCreateStatic(test_task_entry, "s_thr", TASK_STACK_DEPTH, NULL, TASK_PRIORITY,
|
||||
thread_failure_stack, &thread_failure_control_block);
|
||||
txfr_test_account_stop(&counters);
|
||||
txfr_test_check("xTaskCreateStatic returns NULL on thread failure", task == NULL);
|
||||
txfr_test_check_counts("static thread failure unwinds the semaphore", &counters, 0, 0, 2, 1);
|
||||
|
||||
/* The resume is the last step, so by then both kernel objects exist. The
|
||||
thread has to be terminated before it can be deleted, which is the order
|
||||
the idle task uses when it reaps a deleted task. */
|
||||
txfr_test_account_start(TXFR_INJECT_THREAD_RESUME, 1);
|
||||
task = xTaskCreateStatic(test_task_entry, "s_res", TASK_STACK_DEPTH, NULL, TASK_PRIORITY,
|
||||
resume_failure_stack, &resume_failure_control_block);
|
||||
txfr_test_account_stop(&counters);
|
||||
txfr_test_check("xTaskCreateStatic returns NULL on resume failure", task == NULL);
|
||||
txfr_test_check_counts("static resume failure unwinds thread and semaphore", &counters, 0, 0, 2, 2);
|
||||
}
|
||||
|
||||
@@ -52,6 +52,7 @@ UINT __real__txe_thread_create(TX_THREAD *thread_ptr, CHAR *name_ptr, VOID (*ent
|
||||
UINT preempt_threshold, ULONG time_slice, UINT auto_start,
|
||||
UINT thread_control_block_size);
|
||||
UINT __real__txe_thread_delete(TX_THREAD *thread_ptr);
|
||||
UINT __real__txe_thread_resume(TX_THREAD *thread_ptr);
|
||||
|
||||
|
||||
/* Determine whether the call now being made is the one the test asked to fail.
|
||||
@@ -240,6 +241,23 @@ UINT __wrap__txe_thread_delete(TX_THREAD *thread_ptr)
|
||||
}
|
||||
|
||||
|
||||
UINT __wrap__txe_thread_resume(TX_THREAD *thread_ptr)
|
||||
{
|
||||
if(txfr_should_fail(TXFR_INJECT_THREAD_RESUME) != 0)
|
||||
{
|
||||
txfr_counters.thread_resume++;
|
||||
return TX_RESUME_ERROR;
|
||||
}
|
||||
|
||||
if(txfr_accounting != 0)
|
||||
{
|
||||
txfr_counters.thread_resume++;
|
||||
}
|
||||
|
||||
return __real__txe_thread_resume(thread_ptr);
|
||||
}
|
||||
|
||||
|
||||
void txfr_test_account_start(TXFR_INJECT_TARGET target, int fail_on_call)
|
||||
{
|
||||
TX_MEMSET(&txfr_counters, 0, sizeof(txfr_counters));
|
||||
|
||||
@@ -49,7 +49,8 @@ typedef enum TXFR_INJECT_TARGET_ENUM
|
||||
TXFR_INJECT_MUTEX_CREATE,
|
||||
TXFR_INJECT_EVENT_FLAGS_CREATE,
|
||||
TXFR_INJECT_TIMER_CREATE,
|
||||
TXFR_INJECT_THREAD_CREATE
|
||||
TXFR_INJECT_THREAD_CREATE,
|
||||
TXFR_INJECT_THREAD_RESUME
|
||||
} TXFR_INJECT_TARGET;
|
||||
|
||||
/* Counts of the ThreadX primitives the layer reached for while accounting was
|
||||
@@ -70,6 +71,10 @@ typedef struct TXFR_COUNTERS_STRUCT
|
||||
int timer_delete;
|
||||
int thread_create;
|
||||
int thread_delete;
|
||||
|
||||
/* Counted, but not part of the object totals checked below, since a
|
||||
resume creates nothing. */
|
||||
int thread_resume;
|
||||
} TXFR_COUNTERS;
|
||||
|
||||
/* Start accounting. Pass TXFR_INJECT_NONE to only count, or a target together
|
||||
|
||||
@@ -350,6 +350,7 @@ TaskHandle_t xTaskCreateStatic(TaskFunction_t pxTaskCode,
|
||||
ret = tx_thread_create(&pxTaskBuffer->thread, (CHAR *)pcName, txfr_thread_wrapper, (ULONG)pvParameters,
|
||||
puxStackBuffer, stack_depth_bytes, prio, prio, 0u, TX_DONT_START);
|
||||
if(ret != TX_SUCCESS) {
|
||||
(void)tx_semaphore_delete(&pxTaskBuffer->notification_sem);
|
||||
TX_FREERTOS_ASSERT_FAIL();
|
||||
return NULL;
|
||||
}
|
||||
@@ -358,6 +359,12 @@ TaskHandle_t xTaskCreateStatic(TaskFunction_t pxTaskCode,
|
||||
|
||||
ret = tx_thread_resume(&pxTaskBuffer->thread);
|
||||
if(ret != TX_SUCCESS) {
|
||||
/* The thread exists but was never started. It has to be terminated
|
||||
before it can be deleted, which is the order the idle task uses when
|
||||
it reaps a deleted task. */
|
||||
(void)tx_thread_terminate(&pxTaskBuffer->thread);
|
||||
(void)tx_thread_delete(&pxTaskBuffer->thread);
|
||||
(void)tx_semaphore_delete(&pxTaskBuffer->notification_sem);
|
||||
TX_FREERTOS_ASSERT_FAIL();
|
||||
return NULL;
|
||||
}
|
||||
@@ -1489,6 +1496,7 @@ QueueHandle_t xQueueCreateStatic(UBaseType_t uxQueueLength,
|
||||
|
||||
ret = tx_semaphore_create(&pxQueueBuffer->write_sem, "", uxQueueLength);
|
||||
if(ret != TX_SUCCESS) {
|
||||
(void)tx_semaphore_delete(&pxQueueBuffer->read_sem);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user