Little bug in xTaskGenericCreate() ?

I use the LPC2xxx-gcc port with preemtive kernel and dynamic tasks. Sometimes I catched an invalid taskhandle inside the just created task.
I think, this is caused by a tick-interrupt inside the critical section in xTaskGenericCreate(). After the portEXIT_CRITICAL() the new task starts, but the handle (*pxCreatedTask) will not be assigned until the calling task is running again. Please, correct me if I am wrong. signed portBASE_TYPE xTaskGenericCreate( pdTASK_CODE pxTaskCode, const signed char * const pcName, unsigned short usStackDepth, void *pvParameters, unsigned portBASE_TYPE uxPriority, xTaskHandle *pxCreatedTask, portSTACK_TYPE *puxStackBuffer, const xMemoryRegion * const xRegions )
{
… if( pxNewTCB != NULL )
{
… /* We are going to manipulate the task queues to add this task to a
ready list, so must make sure no interrupts occur. */
portENTER_CRITICAL();
{
uxCurrentNumberOfTasks++;
if( uxCurrentNumberOfTasks == ( unsigned portBASE_TYPE ) 1 )
{

}
else
{
/* If the scheduler is not already running, make this task the
current task if it is the highest priority task to be created
so far. */
if( xSchedulerRunning == pdFALSE )
{
if( pxCurrentTCB->uxPriority <= uxPriority )
{
pxCurrentTCB = pxNewTCB;
}
}
} … prvAddTaskToReadyQueue( pxNewTCB ); …
}
portEXIT_CRITICAL();
}
else
{

} if( xReturn == pdPASS )
{
if( ( void * ) pxCreatedTask != NULL )
{
/* Pass the TCB out – in an anonymous way.  The calling function/
task can use this as a handle to delete the task later if
required.*/
*pxCreatedTask = ( xTaskHandle ) pxNewTCB;
} …
} return xReturn;
}

Little bug in xTaskGenericCreate() ?

First, you should NOT be getting tick interrupts inside of a critical section, this would be a major bug in the port. Critical sections should disable every interrupt that might interact with FreeRTOS, and the tic interrupt definitely counts for that. If the problem is that between the portEXIT_CRITICAL and the assigning to pxCreatedTask the created task referes to *pxCreatedTask, that does look like a problem in FreeRTOS. At first glance, maybe the returned value should be set before scheduling the task to run. If you don’t want to change FreeRTOS, a couple of solutions come to mind,
1) Place the create inside of a Critical Section by the calling task. Hopefully this doesn’t impact performance too bad, it does say you have interrupts down longer than you want. 2) Place at the beginning of the created task, assign the handle with xTaskGetCurrentTaskHandle()

Little bug in xTaskGenericCreate() ?

I’m not sure I am following this one. First thing to say is I agree with richard_damon that you should not get tick interrupts inside a critical section (you are not perhaps using a simulator are you?). Other than that, pxCreatedTask and pxNewTCB are both going to be either on the stack or in a register – and should therefore be part of the task context.  If there is something going amiss there then it is in the context switch that the problem lies, and if there is a problem in the context switch then it will show up everywhere (I would assume?). Do you have stack overflow checking switched on? Regards.

Little bug in xTaskGenericCreate() ?

No, I don’t want to change FreeRTOS, it’s fantastic! Many thanks to the developer! I’m not using a simulator, the stacks are fine. I presume, the tick interrupt occurs (and is pending)  within the critical section. Right AFTER portEXIT_CRITICAL() the tick interrupt runs and the scheduler starts the new task if it has a higher priority than the calling task. Meanwhile the assignment to *pxCreatedTask is delayed, and the new task runs for some time without a proper taskhandle. I can see this behavior with the debugger and I’m not sure that this intentionally.

Little bug in xTaskGenericCreate() ?

To clarify this: the problem is not the pxNewTCB which is part of the context, but the the handle whis is returned by xTaskCreate(…, xTaskHandle *pvCreatedTask);

Little bug in xTaskGenericCreate() ?

First – thanks for the kind words. I understand better now the potential issue you are reporting – however I still don’t think it is an issue. The handle to a task is just a pointer to the tasks TCB.  It is optional as to whether you want this information passed out of the xTaskCreate() function or not.  You would want the handle if you were to do something later on, like delete the task, change its priority, suspend it, etc. In the scenario you describe: 1) Task A calls xTaskCreate() to create Task B, and passes in a valid (non NULL) address for pxCreatedTask.
2) During the task creation, in the critical section, a tick interrupt becomes due, but is held pending.
3) As soon as the critical section is exited the tick interrupt executes and selects task B to run.  The newly created task does not know its own task handle but can obtain it through the API call that richard_damon pointed out.  Normally it doesn’t need to know its own handle as it can refer to itself by using a NULL handle in API calls anyway (that is normal practice).  At this point pxCreatedTask is not assigned, but Task A is not running in any case so it does not matter.
4) Task B leaves the Running state and the scheduler (at some point in the future) selects Task A to run again.  Task A starts from the point it left off, so assigns the task handle to pxCreatedTask before it ever gets a change to actually use the value (it happens before it leaves the xTaskCreateFunction). In this scenario there should be no issues.  However, if TaskA passed in the address of a global variable as pxCreatedTask, and Task B tried using the global variable, then there would be a problem – but as I said it would not be normal for a task to use its own handle, it should just use NULL as the handle if it wants to manipulate its own state or delete itself. Regards.

Little bug in xTaskGenericCreate() ?

Richard, while pxCreatedTask will be a local variable *pxCreatedTask may well (in fact I would say often) point to a global. The newly created task could access that variable to attempt to gets its handle for some operations, and in this case it could get an incorrect value. The documentation sort of implies, but doesn’t really promise, the invariant that the location passed will hold the handle of the task for the life of the task (assume the user doesn’t destroy the value himself), but the code really on makes this happen for the calling task without pointing out the limitation. There shouldn’t be a problem with assigning to the passed handle location before enabling the task, getting around the problem, and this makes the invariant what would be expected.

Little bug in xTaskGenericCreate() ?

I have moved the code:
if( ( void * ) pxCreatedTask != NULL )
{
    /* Pass the TCB out - in an anonymous way.  The calling function/
    task can use this as a handle to delete the task later if
    required.*/
    *pxCreatedTask = ( xTaskHandle ) pxNewTCB;
}
to be inside the critical section – immediately above the line xReturn = pdPASS;  This will then be in the next release, which is only a couple of weeks away. This takes care of the global variable case, although the task being created still has the option of just using NULL in place of its task handle. Regards.

Little bug in xTaskGenericCreate() ?

Thanks for bringing that to my attention – by the way. Regards.

Little bug in xTaskGenericCreate() ?

Yes, this is exactly my problem: the new task used the global variable and crashed sometimes. Now I use xTaskGetCurrentTaskHandle() and it works, but I think richard_damon’s suggestion to move the assignment should be better. Thanks a lot.

Little bug in xTaskGenericCreate() ?

Richard, just to point out that I don’t think that code needs to be inside the critical section, but could be outside of (but before) it, after all the location being modified shouldn’t be accessed by anyone else until after the end of the critical section or the code is incorrect anyways.