Software timers: possible race condition in xTimerReset example

Kernel
1

The xTimerReset example code has next funtions:

void vBacklightTimerCallback(TimerHandle_t pxTimer)
{
   vSetBacklightState(BACKLIGHT_OFF);
}

void vKeyPressEventHandler(char cKey)
{
   vSetBacklightState(BACKLIGHT_ON);

   if(xTimerReset(xBacklightTimer, 10) != pdPASS)
   {

   }
}

In case of:

  • preemptive scheduling used
  • timer task has lower priority, than task, calling vKeyPressEventHandler

If vKeyPressEventHandler is called and in the same time, timer became expired, we will turn the backlight on and pass command to reset the timer. If callback will be executed first, we will have a condition when backlight turned off right after button press and the timer is running.

If I understand correctly, in case of timer task having higher priority, than task, calling vKeyPressEventHandler, timer task will resume starting from commands processing, which will update timer expiration time, so callback won’t happen?

So, the example assumes, that timer task has higher priority, than task, calling vKeyPressEventHandler?

2

If I understand correctly, you are describing the following sequence of events -

  1. vKeyPressEventHandler is called and we turn on the LED and reset the timer.
  2. vKeyPressEventHandler is called again after sometime and turns on LED (which is already on).
  3. Timer started in the step 1 expires.
  4. vBacklightTimerCallback gets called and turns off the LED. Note that this can only happen when the timer task has higher priority because vBacklightTimerCallback is called from the timer task.
  5. vKeyPressEventHandler resumes and resets the timer. As a result, the timer is running with the LED off.

You can try to turn on the LED after the timer is reset -

void vKeyPressEventHandler(char cKey)
{
   if(xTimerReset(xBacklightTimer, 10) == pdPASS)
   {
      vSetBacklightState(BACKLIGHT_ON);
   }
   else
   {
      /* Error handling. */
   }
}
3

Hi, thank you for your reply.

Firstly, if timer task has higher priority, there will be no problem, if i correctly understand internals of timer task. Of course, I will need to turn the backlight on only after call to xTimerReset and not before (for some reason, example does it in wrong order?).

Secondly, in case, timer task has lower priority, there clearly is race condition, which isn’t that clear from docs page.

My main concern here is to figure out if my reasoning is right and to maybe ask why there is no any kind of note on docs page (which contains example i took code from) regarding this behaviour, which can actually cause some troubles in real code.

4

That is not correct. The above sequence I described happens when timer task has higher priority.

The following will be the execution sequence when timer task has lower priority:

  1. vKeyPressEventHandler is called and we turn on the LED and reset the timer.
  2. vKeyPressEventHandler is called again after sometime and turns on LED (which is already on).
  3. Timer started in the step 1 expires.
  4. vBacklightTimerCallback DOES NOT get called because timer task has lower priority (note that vBacklightTimerCallback is called from the timer task).
  5. vKeyPressEventHandler keeps on running and resets the timer.
5

In case of timer task having lower priority than second task (which calls vKeyPressEventHandler):

  1. vKeyPressEventHandler is called and we turn on the LED and reset the timer.
  2. vKeyPressEventHandler is called again after sometime and turns on LED (which is already on) + it will enqueue command to reset the timer.
  3. Timer started in the step 1 expires.
  4. vBacklightTimerCallback DOES NOT get called because timer task has lower priority (note that vBacklightTimerCallback is called from the timer task).
  5. vKeyPressEventHandler keeps on running and enqueueing reset commands.
  6. After some time, timer queue will become full and call to xTimerReset will block, which will cause event task to yield to timer task.

But actually, in real world there will be no task which just spins, waiting for event, it will rather block. In that time, timer task can run and execute callback. It means, timers are unusable (in this example) if their task has lower priority.

And even if timer task has higher priority, we have to rely on reading timer task internals to understand what kind of behavior to expect. As there can be both timer expiration and reset commands pending at the same time (in rare cases of course), so we have to know the order of processing of those.

6

You mean to say that the timer task can run the callback and turn off the LED after vKeyPressEventHandler has turned off the LED and before it has submitted the reset command? If yes, that can only happen if the vKeyPressEventHandler task blocks after turning of the LED or the timer task has a higher priority.

If a timer has expired before the timer task picks and processes the RESET command, then the timer callback is invoked and then the RESET command is processed.

This sample code is just to explain the xTimerReset API and you are free to use it whichever way it suits your application.

7

Yes, that’s exactly what i meant.

Why so? If we use preemptive scheduling and priority of timer task is lower, any other task (which has higher priority) can preempt timer task on tick interrupt:

  1. timer started
  2. timer task started to check timers expiration
  3. timer task got preempted
  4. event task calls vKeyPressEventHandler
  5. event task enables LED
  6. event task sends reset command
  7. event task blocks
  8. timer task continues
  9. timer task calls callback
  10. timer task disables LED
  11. timer task resets timer

This is exactly what you are talking about here:

In that case sample code works wrong sometimes: it disables LED right on button press or rather doesn’t re-enable it.

I understand that that is just sample code, it just happened so, my use case of timers is close in theory, just a bit more strict: i’m trying to implement some kind of non-blocking lock with auto-unlock on timeout and random unlocks can be really dangerous.

8

One thing to remember is that timers operate through a command queue, and thus operations on them are somewhat asynchronous. especially if the timer task is not the highest priority task.

It seems your design has an essential race between the turn on and the turn off events which you need to handle in your design since the operation of turning off the LED is not an atomic operation from the decision that it is time to turn it off to the point of actualy turning it off.

You may need to use a critical section and a status variable that the call back can check atomically in the critical section to verify it does want to turn off the LED.

9

Hi,

The problem is, even if you consider LED switch an atomic operation (or wrap it using critical section), you just can sometimes get event callback, that enabled LED and right after that, timer callback will fire, which will turn the LED off, after what the timer will be reset and run while LED is already OFF. And your code will think, that LED is ON.

I thought of some simple solution using flags, but either I’m dumb, or it doesn’t exist by design.

This is ok-ish in case of LEDs (nobody will complain if 1 in 10000000 time, they work wrong), but in case of locks, that protect some critical memory, it can be vital.

I think, in case of timer task having higher priority, we are guaranteed to yield from timer task from one single place (correct me if I’m wrong) and that guarantees us strict order of commands processing vs expiration time checks (as I see from code, commands will be processed first).

10

Actually, maybe this workaround will work:

void vBacklightTimerCallback(TimerHandle_t pxTimer)
{
   vSetBacklightState(BACKLIGHT_OFF);
}

void vPendableFunction(void * pvParameter1, uint32_t ulParameter2)
{
   TimerHandle_t timer = pvParameter1;

   if(xTimerIsTimerActive(timer) == pdFALSE)
      vSetBacklightState(BACKLIGHT_ON);

   xTimerReset(timer, 0);
}

void vKeyPressEventHandler(char cKey)
{
   xTimerPendFunctionCall(vPendableFunction, xBacklightTimer, 0, portMAX_DELAY);
}

Running event callback in timer task context will guarantee order of operations.

11

My thought is when you turn on the LED, you set a flag to the current time stamp. The turn off code, inside a critical section checks if that is too recent, and if so, doesn’t turn it off.

12

You are right and it is a general practice to have the timer task at the highest priority.

13

@aggarg,

So, to dot the i’s:

  1. Timer task should always have higher priority, than any task, that somehow uses timers. Moreover, it better to have the highest priority to be safe from mutex’s priority inheritance. It’s the easiest and the safest approach.
  2. We can safely assume that in case of timer task having the highest priority in our application, the order of timer expiration checks and commands processing by the timer task is determined. And, which is crucial, in case there are both expiration and command pending, the first to be processed will always be command.
  3. The example is still broken even with timer task having the highest priority. Turning the backlight LED on before reset has the chance of task calling vKeyPressEventHandler beeing preempted right after turning the backlight LED on (we assume, that turning it on/off is an atomic operation, otherwise, we also need a critical section) by the timer task and timer callback beeing called which will disable the LED right after we enabled it. Backlight LED should only be turned on after reset:
void vKeyPressEventHandler( char cKey )
{
    if( xTimerReset( xBacklightTimer, 10 ) == pdPASS )
    {
        vSetBacklightState( BACKLIGHT_ON );
    }
}
14

Best not to assume this implementation of FreeRTOS timers. It’s not part of the API or documentation, and I believe there are already corner cases that violate the assumption.

15

Hi,

Yeah, I understand that, but in this case I can’t see any other way to get guaranteed deterministic behavior. I guess, there is still this option I mentioned:

16

@Satty Looks like that solution works. In general, careful usage of the timer task’s own context can synchronize the work and manage the races correctly.

However, if the timer task has higher priority than your other tasks, then your other suggested fix is sufficient on single-core and AMP systems:

With either of these solutions, there is no need to assume the timer task processes commands and expirations in any particular order.

17

There is still a problem I see with this solution, you mentioned in case of undefined order of commands processing vs expirations checks:

Let’s imagine we have a task, that is currently calling vKeyPressEventHandler. If right at the same time our timer will expire, nothing will happen as we have to preempt to timer task to check for that and run timer callback routine. This will only happen in case of tick interrupt happening or, what is more likely in our case, when we call xTimerReset (which will unblock the timer task by enqueueing reset command). This way we have both expiration and command pending and in case of undefined order there are 2 possible outcomes. The problem with those is that you can’t check what order you got. Either you got reset first and no callback (as reset will hold it over for another timer period) or you will first get callback and then the timer will be re-started (as xTimerReset can also do that).

I’m assuming we are talking about single-core systems.

18

Yes, but both outcomes are OK. (Or did I miss something?)

Note the timer task does preempt other tasks when a timer expires. Remember that timers expire when the system reaches a specific tick count.

19

They are not. In case of callback being executed first, example will work incorrectly: it will enable LED in vKeyPressEventHandler, then the callback will disable it and the timer will be reset (which gives us very weird total state of the system). This way the LED will be off right after vKeyPressEventHandler call.

And as I mentioned earlier:

That’s right but tick intrrupt can still happen after we call xTimerReset and switch to timer task which will give us both command and expiration pending.

20

My bad, i forgot about updated code.

With vSetBacklightState after xTimerReset, both orders will work fine.