Bug found in xTimerChangePeriod()

bakerstu wrote on Friday, September 27, 2013:

I checked the latetest 7.5.2 and this potential bug still persists. It is possible that there is a race when changing the timer period that the timer has already expired. Though when starting/restarting the timer, this is caught by looking at the return value for prvInsertTimerInActiveList(), the return value of prvInsertTimerInActiveList() is not checked in the case of a change in period.

Would someone be able to confirm my findings?

Thanks,
Stuart

rtel wrote on Friday, September 27, 2013:

I will look at this today - but I would ask you don’t post a message saying “bug found” until you know there is a bug found. It is a very definitive statement which will make people subscribing to this forum assume they are being informed of something, yet if you look through the support archive you will find most (all?) “bug in nnn” posts turn out not to be. “possible bug found” would be a more accurate subject until we know whether it is a bug or not.

…I will post again with my findings once this has been looked at.

Regards.

rtel wrote on Friday, September 27, 2013:

  1. Looking at the case where a timer is started, which handles the return value of prvInsertTimerInActiveList():

When xTimerStart() is called a message is sent to the timer task. The xMessageValue member of the message is set to the tick count to mark the time at which the message was sent. When the message is handled prvInsertTimerInActiveList() is called with xNextExpiryTime calculated as the time the message was sent plus the period, and xCommandTime set to the time the message was sent. These two parameters are used to determine if the timer expired between the message being sent and the message being processed.

  1. Looking at the case you highlight where the period of a time is changed. Semantically it is difficult to know what changing the period of a timer means - relative to what because the new period could be longer or shorter than the old period? In this case changing the period of a timer causes the timer to restart afresh.

When xTimerChangePeriod() is called a message is sent to the timer task. The xMessageValue member of the message is set to the new period value (note this was not the case when the timer was started). When the message is handled prvInsertTimerInActiveList() is called with xNextExpireTime set to the current time plus the timer new period, and xCommandTime set to the current time. Therefore the next expiry time can only be in the future because zero is not a valid timer period - as the expiry time can only be in the future prvInsertTimerInaCtiveList() will never return without placing the timer in a list, so there is no fail case to handle.

Do you agree with this? If so, I don’t think there is a bug, but please let me know if you are thinking of a different scenario.

Regards.

bakerstu wrote on Saturday, October 05, 2013:

So what I’m concerned about is that that the period is changed relative to “now” when I would expect the period to be changed relative to when the timer was first started, and a check to be made if the new period is in the past and already expired. In this way, there is no “drift” in the timer experation based on the original start time.

Perhaps an enhnancement request is one way to resolve this to provide the user with the option on what the change period experation time is relative to.

Thanks,
Stuart

P.S. Sorry about the posting headline, I’ll note this for the future. If you have the ability to change the headline, that is alright with me.