[Tinyos-devel] Bug in timer system

[Jan Hauer]

I think we've found a bug in the timer system
(tos/lib/timer/VirtualizeTimerC.nc). It shows up when you use two
Timer interfaces (e.g. provided by two TimerMilliC() instances) and
Timer2.startOneShot() is called withinTimer1.fired() and in addition
(1) Timer2 has a higher internal interface ID (unique(UQ_TIMER_MILLI))
than Timer1 and (2) there is some (small) amount of time elapsed
before the Timer2.startOneShot() is called in the fired eventhandler.
Then, Timer2.fired() is signalled immediately independent of the "dt"
parameter in startOneShot(uint32_t dt).

The problem is function "fireTimers(uint32_t now)" starting from line
62 in tos/lib/timer/VirtualizeTimerC. Here, the for-loop scans through
all requests and makes a decision whether a timer is due based on a
"now" parameter that is passed as a parameter to fireTimers. After it
has signalled the fired() event it wrongly continues to use "now" as
reference time. The problem is that if within fired() a different
timer was set (e.g. via startOneShot()) then its t0 may be larger than
the "now" parameter. But then this timer will be interpreted as lying
in the past and its fired() will be signalled immediately. The bug can
be reproduced with a small modification to apps/BlinkC shown below(*).
The suggested fix is to insert a break statement after the line 81
"signal Timer.fired[num]();".

The bug is not present in tos/lib/timer/VirtualizeAlarmC.nc, but there
is a different issue with this component on msp430: with mspgcc-3.2.3
I get a "Segmentation fault" (see error message below (**)) during
compilation if I use uint32_t as "size_type", e.g.
"VirtualizeAlarmC(T32khz, uint32_t, uniqueCount(UQ_ALARM_32KHZ32));".
The probelm is in line 135 of VirtualizeAlarmC. Currently lines
134-136 read:

if( m.isset[id] ) {
  size_type elapsed = call AlarmFrom.getNow() - m.alarm[id].t0;
  if( m.alarm[id].dt <= elapsed ) {

I don't understand the actual cause, but when I replace this with the
following code it compiles:

 if( m.isset[id] ) {
  size_type t0 = m.alarm[id].t0;
  size_type elapsed = call AlarmFrom.getNow() - t0;
    if( m.alarm[id].dt <= elapsed ) {

This fix should not have any impacts for other platforms, so I suggest
to use it as a workaround.

Jan

(*) To reproduce the bug in  VirtualizeTimerC.nc replace the BlinkC
implementation section with the following code. Led1 should flash 5s
after Led0, but they both flash immediately (tested on TelosB, other
platforms may require longer busy wait).

  event void Boot.booted(){
    call Timer0.startOneShot( 250 );
  }

  event void Timer0.fired(){
    uint32_t i,j=2;
    for (i=0; i<1000; i++) // spent some time
      j *= j;
    call Leds.led0Toggle();
    call Timer1.startOneShot( 5000 );
  }

  event void Timer1.fired(){
    call Leds.led1Toggle();
  }

  event void Timer2.fired() {}

(**) The following shows the mspgcc error I get when I instantiate
"VirtualizeAlarmC(T32khz, uint32_t, uniqueCount(UQ_ALARM_32KHZ32));"
The error message wrongly refers to the usart implementation (because
of nesc inlining, I guess).

/home/hauer/tos/tinyos-2.x/tos/chips/msp430/usart/HplMsp430UsartInterrupts.nc:
In function `VirtualizeAlarmC$0$signalAlarms'
:
/home/hauer/tos/tinyos-2.x/tos/chips/msp430/usart/HplMsp430UsartInterrupts.nc:49:
internal error: Segmentation fault