Fixed the kernel object leaks on the static creation error paths - #584
Merged
fdesbiens merged 1 commit intoAug 9, 2026
Merged
Conversation
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two sibling leaks noted in #582, now with test coverage from #583 rather than by inspection.
The defect
xQueueCreateStatic()andxTaskCreateStatic()take their storage from the caller, so neither leaks memory. Both still create ThreadX objects, and both still returnNULLwhen a later step fails. The caller is left without a handle and cannot call the matching delete function, so an object already created stays registered in the kernel, pointing into a caller buffer the application is now free to reuse or discard.Three paths were affected:
xQueueCreateStaticwrite_semcreationread_semxTaskCreateStatictx_thread_createnotification_semxTaskCreateStatictx_thread_resumenotification_semand the threadThe third one was not in the original list; it turned up while writing the tests for the second.
The fix
Each path now deletes what it had already created. The resume path terminates the thread before deleting it, since a thread created with
TX_DONT_STARTis suspended rather than terminated andtx_thread_delete()would otherwise refuse it. That is the same order the idle task uses when it reaps a deleted task, so the layer is consistent with itself.Eight lines of product code in total.
Verification
Six new checks, three of which are the paths above. The suite from #583 gained thread resume as an injectable entry point to reach the last one.
Against the layer as it stands on
dev, exactly the three new count checks fail, and they name what was abandoned:With the fix, all three tests pass.
Each static failure case now uses its own control block. Sharing one made the numbers lie: on the unfixed layer, the semaphore abandoned by the thread-failure case poisoned the buffer the next case cleared and reused, so that case reported
create=1instead ofcreate=2and pointed at the wrong thing. Independent buffers mean a future regression on one path cannot mislead the diagnosis of another.One thing left alone
xTaskCreate()handles atx_thread_resume()failure differently from the static variant it otherwise mirrors: it callsTX_FREERTOS_ASSERT_FAIL(), then increments the task count and returnspdPASS. The caller is told the task started when it did not, and holds a handle to a task that will never run.That is a behavioural question rather than a leak — deciding whether the resume failure should propagate means deciding what a caller can expect from a task that exists but is not scheduled — so it is out of scope here.