Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMSIS layer never free()'s memory associated with a thread that has been detached and terminated #48

Open
RobMeades opened this issue Apr 20, 2024 · 7 comments
Assignees
Labels
bug Something isn't working internal bug tracker Issue confirmed and logged into the internal bug tracking system mw Middleware-related issue or pull-request. rtos Real-Time Operating System spotted before customer Spotted internally before being pointed out by the user but not yet fixed or published

Comments

@RobMeades
Copy link

RobMeades commented Apr 20, 2024

We are using ThreadX through the CMSIS2 layer provided here with USE_DYNAMIC_MEMORY_ALLOCATION.

When we delete a task, we call osThreadDetach() and then osThreadTerminate(), expecting this to free the memory that was allocated when we called osThreadNew(); however this is never the case, and we soon run out of memory.

You can see this if you set break-points here:

osStatus_t osThreadTerminate(osThreadId_t thread_id)

...here:

_tx_thread_system_preempt_check();

...and here:

if (thread_ptr->tx_thread_detached_joinable == osThreadDetached)

The first two break points are reached when osThreadTerminate() is called, status is 0 at the second break-point but, the last break-point is never reached, presumably because _tx_thread_system_preempt_check() has done its stuff, task-switching has occurred, the code that follows the call to tx_thread_terminate() is never going to be run.

How is this meant to work?

FYI, we have tried calling tx_thread_preemption_change() and tx_thread_priority_change() with zero before terminating that thread, to set the current thread to top priority (0 in ThreadX terms), which this post suggests should effectively create a critical section, but that doesn't change the behaviour, _tx_thread_system_preempt_check() still chooses to switch to what is now a lower-priority (54 in ThreadX terms) thread.

This tested on a Nucleo STM32F575ZI board, in case it matters.

@ALABSTM ALABSTM added question Further information is requested mw Middleware-related issue or pull-request. rtos Real-Time Operating System labels Apr 22, 2024
@ALABSTM ALABSTM added bug Something isn't working and removed question Further information is requested labels Apr 22, 2024
@RobMeades
Copy link
Author

Any thoughts on this? I would like to publish the code to customers next week.

@ALABSTM ALABSTM assigned ALABSTM and unassigned TOUNSTM May 3, 2024
@ALABSTM ALABSTM added the internal bug tracker Issue confirmed and logged into the internal bug tracking system label May 3, 2024
@ALABSTM
Copy link
Contributor

ALABSTM commented May 3, 2024

ST Internal Reference: 180641

@ALABSTM
Copy link
Contributor

ALABSTM commented May 3, 2024

Hi @RobMeades,

Please excuse this delayed reply. Thank you for the detailed report. It has been forwarded to our development teams. We will try our best to get back to you by next week.

With regards,

@ALABSTM ALABSTM added the spotted before customer Spotted internally before being pointed out by the user but not yet fixed or published label May 6, 2024
@ALABSTM
Copy link
Contributor

ALABSTM commented May 6, 2024

Hi @RobMeades,

Our development teams acknowledged the issue and said they are already aware of it. They are working on a solution. Unfortunately, they still need time and it will probably not be for this week. They cannot share a date for the moment.

I will keep you informed should there be any update. We apologize for the inconvenience and count on your patience and comprehension.

With regards,

@RobMeades
Copy link
Author

@ALABSTM: understood, thanks for the update; as soon as you have a rough date let me know and I will let our customers know.

RobMeades added a commit to u-blox/ubxlib that referenced this issue May 9, 2024
This commit adds the I2C and SPI support that was missing in the previous commit.

Note that ST have acknowledged the problem with the ST-provided CMSIS layer for ThreadX not freeing memory when a dynamic task is terminated: they are looking into a fix, no timescale yet though; this is being progressed in STMicroelectronics/STM32CubeU5#48.
@RobMeades
Copy link
Author

Any update on this one (one of our customers is asking)?

@RobMeades
Copy link
Author

<bump />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal bug tracker Issue confirmed and logged into the internal bug tracking system mw Middleware-related issue or pull-request. rtos Real-Time Operating System spotted before customer Spotted internally before being pointed out by the user but not yet fixed or published
Projects
Status: In progress
Development

No branches or pull requests

3 participants