Re-using static allocated TCB is not trivial

This issue already posted in the past in this forum (Re-using static TCB race condition), using xTaskCreateStatic one shouldn’t re-use the TCB static memory if the task deleted itself (see in vTaskDelete the comment before calling prvDeleteTCB) till the idle task ran and removed it from the task lists, elsewhere when the idle task may be removing afterward and may crash the system.
My suggestions are:

  1. I think such thing should be documented in bold. It is critical to user to know this restriction. (debugging what happened after such crash is very hard)
  2. For me it sounds like user should have an API to force cleanup TCB (also in dynamic allocation to force releasing the memory is probably good idea)
  3. I suggest in prvCheckTasksWaitingTermination to add assert that ensure (as much as can) that the terminated TCB that we are removing is not re-used. Add:
    a. configASSERT((void *)pxTCB->xStateListItem.pxPrevious == (void *)&xTasksWaitingTermination.xListEnd);
    b. configASSERT((void *)pxTCB->xStateListItem.pxNext->pxPrevious == (void *)&pxTCB->xStateListItem);
    after getting the head of the terminated list but before removing it.

Any reason why the current documentation doesn’t fit the need? The note does mention that the idle task must run for cleanup to occur. Maybe the wording could be improved?

I think this is a neat suggestion. We could probably add some kind of force or immediate flag to vTaskDelete to ensure cleanup were to happen immediately. Let me discuss with the team.

Why prohibit TCB memory reuse if you’ve forced the cleanup of the TCB? Or is this check something that would only apply before termination is completed?

My first thought is why are you doing this in the first place. If you have statically reserved the memory for the TCB and Stack, there is no reason to have the task delete itself and then create it again, you can just as easily have the task itself just block instead of deleting, and then resume instead of creating the new copy.

The fact that the resources were statically allocated doesn’t remove the need to “clean them up” it just removes one of the steps.

The problem I can see it an API to force the immediate deallocation is that it couldn’t be done in the task that it deleting itself, as that case needs the task to still exist, to run the code to do the cleanup, but the task has to not exist as something that can be executed to be ready to do that. There is a fundamental race here.

For another task to try to force the cleanup to happen in its task environment runs into the problem that if the idle task has started, but not completed, the cleanup operation, then the task can claim that operation, as again, we are stuck in a race condition.

The real answer to the problem would seem to be for the task not to delete itself, but send a message to a “reaper” task asking to be deleted, and then suspend/block forever (until it is deleted). This allows the user to track whatever they want to confirm the order of deletion.

Yes, there are asserts that could be used to detect a lot of the cases, but I doubt there is a race free and reliable method.

Why do you want to delete a statically allocated task? If you want to immediately release the resources, you need to delete the task from a different task and @richard-damon’s suggestion of using a “reaper” task should solve your problem.

Thanks for the replies,
OK, I agree that the documentation is OK, my mistake.
About cleanup, I understand that a task can’t clean itself, I mean that other task will force cleanup, indeed current prvCheckTasksWaitingTermination function can’t be accessed by multy threads, if will decide to clean from other task will need to add more code under the enter critical.
About the asserts, it is probably depends how many asserts you like in the kernel.

My use case where I bumped into this issue was in tests, where each time I started new test as a new task and gave it the test function to run.
I already fixed my code to never kill tasks, just send the test functions to worker thread.
I just trying to think how to avoid such issues in the future.

Glad that you are able to make it work!

2 Likes

I like your suggestion richard-damon. The only other hack I could think of (and it’s an ugly hack) would be to have the idle task inherit the priority of the self-destructing task until it was able to complete clean up. Once it did the idle task would then drop back to its regular priority.

I wasn’t able to think through this fully before you responded but there are massive downsides to this (breaking the guarantee of when the idle task runs, timing issues, idle hook issues).

Yes, changing the priority of Idle would break all sorts of expectations. Adding a “Reaper” task gets around the issues. I suppose there could be a FreeRTOSconfig.h option to enable it, and if enabled make vTaskDelete(NULL) automatically use it to delete itself.

Doing something like memsetting the tcb to zeros upon task clean up would at least give a programmatic way of knowing if the idle task had done its thing.

1 Like

Yes, doing something definitively detectable could let eTaskGetState to distinguish between “Scheduled for cleanup” and has been cleaned up.

I might suggest something lighter weight and atomic, like setting pxTopOfStack to NULL as the last step, and checking that. (Don’t know of any processors usable with FreeRTOS where a current stack pointer of 0 is valid case.)

1 Like

Let’s assume for a moment that the OP does want to delete a task, then re-allocate the statically allocated TCB.

One solution I can think of, and please correct me if I am wrong, but the application can wait on a flag to indicate the task is deleted, and cleanup has not been performed.

In the idle task hook, the flag is cleared. In case the cleanup is performed by the idle task after the hook, the flag could simply get cleared on the 2nd iteration of the idle task.

This “flag” can be a simple integer, but it could also be a direct-to-task notification which the calling task could block on, and the idle task hook can notify it with vTaskNotifyGiveIndexedFromISR.

1 Like