Ticket #147: pr147.diff

new file c/src/exec/rtems/changes

@@ +1 @@
+2001-03-29      Joel Sherrill <joel@OARcorp.com>
+
+        * Per PR147 addressed problems when reseting and inserting a timer
+        into a timer chain that did not honor time passage since the last
+        time the timer server was scheduled and the new insertion.
+        * include/rtems/rtems/timer.h, src/timerreset.c, src/timerserver.c,
+        src/timerserverfireafter.c, src/timerserverfirewhen.c: Broke up
+        the "reset server" routine into a set of very specific routines
+        that allowed the server to be unscheduled, timer chains to be
+        "synchronized" with the current time before inserting a new timer.
+

new file c/src/tests/sptests/changes

@@ +1 @@
+2001-03-29      Joel Sherrill <joel@OARcorp.com>
+
+        * Per PR147 addressed problems when reseting and inserting a timer
+        into a timer chain that did not honor time passage since the last
+        time the timer server was scheduled and the new insertion.
+        * sp31/sp31.scn, sp31/task1.c: Added test code to detect this case.
+

c/src/exec/rtems/include/rtems/rtems/timer.h

@@ -135 +135 @@
 );
 
 /*
- *  _Timer_Server_reset
- *
- *  DESCRIPTION:
- *
- *  This routine resets the timers which determine when the Timer Server
- *  will wake up next to service task-based timers.
- */
-
-typedef enum {
-  TIMER_SERVER_RESET_TICKS,
-  TIMER_SERVER_RESET_SECONDS
-} Timer_Server_reset_mode;
-
-void _Timer_Server_reset(
-  Timer_Server_reset_mode  reset_mode
-);
-
-/*
  *  rtems_timer_create
  *
  *  DESCRIPTION:
@@ -338 +320 @@
   Objects_Id               id,
   rtems_timer_information *the_info
 );
+
+/*
+ *  Macros and routines that expose the mechanisms required to service
+ *  the Timer Server timer.  These stop the timer, synchronize it with
+ *  the current time, and restart it.
+ */
+
+extern Watchdog_Control _Timer_Seconds_timer;
+
+#define _Timer_Server_stop_ticks_timer() \
+      _Watchdog_Remove( &_Timer_Server->Timer )
+
+#define _Timer_Server_stop_seconds_timer() \
+      _Watchdog_Remove( &_Timer_Seconds_timer );
+
+void _Timer_Server_process_ticks_chain(void);
+void _Timer_Server_process_seconds_chain(void);
+
+#define _Timer_Server_reset_ticks_timer() \
+   do { \
+      if ( !_Chain_Is_empty( &_Timer_Ticks_chain ) ) { \
+        _Watchdog_Insert_ticks( &_Timer_Server->Timer, \
+           ((Watchdog_Control *)_Timer_Ticks_chain.first)->delta_interval ); \
+      } \
+   } while (0)
+
+#define _Timer_Server_reset_seconds_timer() \
+   do { \
+      if ( !_Chain_Is_empty( &_Timer_Seconds_chain ) ) { \
+        _Watchdog_Insert_seconds( &_Timer_Seconds_timer, \
+          ((Watchdog_Control *)_Timer_Seconds_chain.first)->delta_interval ); \
+      } \
+   } while (0)
 
 #ifndef __RTEMS_APPLICATION__
 #include <rtems/rtems/timer.inl>

c/src/exec/rtems/src/timerreset.c

@@ -57 +57 @@
           _Watchdog_Insert( &_Watchdog_Ticks_chain, &the_timer->Ticker );
           break;
         case TIMER_INTERVAL_ON_TASK:
+          _Timer_Server_stop_ticks_timer();
           _Watchdog_Remove( &the_timer->Ticker );
+          _Timer_Server_process_ticks_chain();
           _Watchdog_Insert( &_Timer_Ticks_chain, &the_timer->Ticker );
+          _Timer_Server_reset_ticks_timer();
           break;
         case TIMER_TIME_OF_DAY:
         case TIMER_TIME_OF_DAY_ON_TASK:

c/src/exec/rtems/src/timerserver.c

@@ -49 +49 @@
 /*
  *  The timer used to control when the Timer Server wakes up to service
  *  "when" timers.
- *
- *  NOTE: This should NOT be used outside this file.
  */
 
 Watchdog_Control _Timer_Seconds_timer;
 
-/*
- *  prototypes for support routines to process the chains
- */
-
-void _Timer_Process_ticks_chain(void);
-void _Timer_Process_seconds_chain(void);
-
 /*PAGE
  *
  *  _Timer_Server_body
@@ -97 +88 @@
      */
 
       _Thread_Set_state( _Timer_Server, STATES_DELAYING );
-      _Timer_Server_reset( TIMER_SERVER_RESET_TICKS );
-      _Timer_Server_reset( TIMER_SERVER_RESET_SECONDS );
+      _Timer_Server_reset_ticks_timer();
+      _Timer_Server_reset_seconds_timer();
     _Thread_Enable_dispatch();
 
+    /*
+     *  At this point, at least one of the timers this task relies 
+     *  upon has fired.  Stop them both while we process any outstanding
+     *  timers.  Before we block, we will restart them.
+     */
+ 
+      _Timer_Server_stop_ticks_timer();
+      _Timer_Server_stop_seconds_timer();
 
     /*
      *  Disable dispatching while processing the timers since we want 
@@ -110 +109 @@
      */
 
     _Thread_Disable_dispatch();
-      _Timer_Process_ticks_chain();
-      _Timer_Process_seconds_chain();
+      _Timer_Server_process_ticks_chain();
+      _Timer_Server_process_seconds_chain();
   }
 }
 
@@ -244 +243 @@
 }
 
 /*PAGE
- *
- *  _Timer_Server_reset
- *
- *  This routine resets the timers which determine when the Timer Server
- *  will wake up next to service task-based timers.
- *
- *  Input parameters:
- *    do_ticks - TRUE indicates to process the ticks list
- *               FALSE indicates to process the seconds list
- *
- *  Output parameters:  NONE
- */
-
-void _Timer_Server_reset(
-  Timer_Server_reset_mode reset_mode
-)
-{
-  Watchdog_Interval  units;
-
-  switch ( reset_mode ) {
-    case TIMER_SERVER_RESET_TICKS:
-      _Watchdog_Remove( &_Timer_Server->Timer );
-      _Timer_Process_ticks_chain();
-      if ( !_Chain_Is_empty( &_Timer_Ticks_chain ) ) {
-        units = ((Watchdog_Control *)_Timer_Ticks_chain.first)->delta_interval;
-        _Watchdog_Insert_ticks( &_Timer_Server->Timer, units );
-      }
-      break;
-    case TIMER_SERVER_RESET_SECONDS:
-      _Watchdog_Remove(  &_Timer_Seconds_timer );
-      _Timer_Process_seconds_chain();
-      if ( !_Chain_Is_empty( &_Timer_Seconds_chain ) ) {
-        units = ((Watchdog_Control *)_Timer_Seconds_chain.first)->delta_interval;
-        _Watchdog_Insert_seconds( &_Timer_Seconds_timer, units );
-      }
-      break;
-  }
-}
-
-/*PAGE
  *
- *  _Timer_Server_Process_ticks_chain
+ *  _Timer_Server_process_ticks_chain
  *
  *  This routine is responsible for adjusting the list of task-based 
  *  interval timers to reflect the passage of time.
@@ -295 +254 @@
  *  Output parameters:  NONE
  */
 
-void _Timer_Process_ticks_chain(void)
+void _Timer_Server_process_ticks_chain(void)
 {
   Watchdog_Interval snapshot;
   Watchdog_Interval ticks;
@@ -312 +271 @@
 
 /*PAGE
  *
- *  _Timer_Server_Process_seconds_chain
+ *  _Timer_Server_process_seconds_chain
  *
  *  This routine is responsible for adjusting the list of task-based 
  *  time of day timers to reflect the passage of time.
@@ -322 +281 @@
  *  Output parameters:  NONE
  */
 
-void _Timer_Process_seconds_chain(void)
+void _Timer_Server_process_seconds_chain(void)
 {
   Watchdog_Interval snapshot;
   Watchdog_Interval ticks;

c/src/exec/rtems/src/timerserverfireafter.c

@@ -69 +69 @@
       the_timer->the_class = TIMER_INTERVAL_ON_TASK;
       _Watchdog_Initialize( &the_timer->Ticker, routine, id, user_data );
       the_timer->Ticker.initial = ticks;
+
+      _Timer_Server_stop_ticks_timer();
+      _Timer_Server_process_ticks_chain();
       _Watchdog_Insert( &_Timer_Ticks_chain, &the_timer->Ticker );
-      _Timer_Server_reset( TIMER_SERVER_RESET_TICKS );
+       _Timer_Server_reset_ticks_timer();
+
       _Thread_Enable_dispatch();
       return RTEMS_SUCCESSFUL;
   }

c/src/exec/rtems/src/timerserverfirewhen.c

@@ -77 +77 @@
       the_timer->the_class = TIMER_TIME_OF_DAY_ON_TASK;
       _Watchdog_Initialize( &the_timer->Ticker, routine, id, user_data );
       the_timer->Ticker.initial = seconds - _TOD_Seconds_since_epoch;
+
+      _Timer_Server_stop_seconds_timer();
+      _Timer_Server_process_seconds_chain();
       _Watchdog_Insert( &_Timer_Seconds_chain, &the_timer->Ticker );
-      _Timer_Server_reset( TIMER_SERVER_RESET_SECONDS );
+       _Timer_Server_reset_seconds_timer();
+
       _Thread_Enable_dispatch();
       return RTEMS_SUCCESSFUL;
   }

c/src/tests/sptests/sp31/sp31.scn

@@ -1 +1 @@
 *** TEST 31 ***
 INIT - rtems_timer_create - creating timer 1
 INIT - timer 1 has id (0x14010001)
+INIT - rtems_timer_create - creating timer 2
+INIT - timer 2 has id (0x14010002)
 TA1 - rtems_timer_ident - identing timer 1
 TA1 - timer 1 has id (0x14010001)
-TA1 - rtems_clock_get - 09:00:00   12/31/1988
+TA1 - rtems_timer_ident - identing timer 2
+TA1 - timer 2 has id (0x14010002)
+TA1 - rtems_timer_server_fire_after - 1 second
+TA1 - rtems_task_wake_after - 1/2 second
+TA1 - rtems_timer_server_fire_after - timer 2 in 1/2 second
+TA1 - rtems_timer_cancel - timer 1
+TA1 - rtems_timer_cancel - timer 2
+TA1 - rtems_timer_server_fire_after - timer 1 in 30 seconds
+TA1 - rtems_timer_server_fire_after - timer 2 in 60 seconds
+Timer 1 scheduled for 3050 ticks since boot
+Timer Server scheduled for 3050 ticks since boot
+TA1 - rtems_task_wake_after - 1 second
+TA1 - rtems_timer_server_fire_after - timer 2 in 60 seconds
+Timer 1 scheduled for 3050 ticks since boot
+Timer Server scheduled for 3050 ticks since boot
+TA1 - rtems_task_wake_after - 1 second
+TA1 - rtems_timer_server_fire_after - timer 2 in 60 seconds
+Timer 1 scheduled for 3050 ticks since boot
+Timer Server scheduled for 3050 ticks since boot
+TA1 - rtems_timer_cancel - timer 1
+TA1 - rtems_timer_cancel - timer 2
+TA1 - rtems_clock_get - 09:00:02   12/31/1988
 TA1 - rtems_timer_server_fire_after - timer 1 in 3 seconds
 TA1 - rtems_task_suspend( RTEMS_SELF )
-TA1 - rtems_clock_get - 09:00:03   12/31/1988
+TA1 - rtems_clock_get - 09:00:05   12/31/1988
 TA1 - rtems_timer_server_fire_after - timer 1 in 3 seconds
 TA1 - rtems_task_wake_after - 1 second
-TA1 - rtems_clock_get - 09:00:04   12/31/1988
+TA1 - rtems_clock_get - 09:00:06   12/31/1988
 TA1 - rtems_timer_reset - timer 1
 TA1 - rtems_task_suspend( RTEMS_SELF )
-TA1 - rtems_clock_get - 09:00:07   12/31/1988
+TA1 - rtems_clock_get - 09:00:09   12/31/1988
 <pause>
 TA1 - rtems_timer_server_fire_after - timer 1 in 3 seconds
 TA1 - rtems_timer_cancel - timer 1
@@ -26 +49 @@
 TA1 - rtems_timer_cancel - timer 1
 TA1 - rtems_task_wake_after - YIELD (only task at priority)
 TA1 - timer_deleting - timer 1
-*** END OF TEST 31 ***
+*** END OF TEST 31 *** 

c/src/tests/sptests/sp31/task1.c

@@ -15 +15 @@
  *  found in the file LICENSE in this distribution or at
  *  http://www.OARcorp.com/rtems/license.html.
  *
- *  $Id: task1.c,v 1.1.2.2 2002/01/29 18:20:46 joel Exp $
+ *  $Id: task1.c,v 1.3 2002/01/29 18:20:59 joel Exp $
  */
 
 #include "system.h"
 
+volatile int TSR_fired;
+
+rtems_timer_service_routine Should_not_fire_TSR(
+  rtems_id  ignored_id,
+  void     *ignored_address
+)
+{
+  TSR_fired = 1;
+}
+
 rtems_task Task_1(
   rtems_task_argument argument
 )
@@ -42 +52 @@
   directive_failed( status, "rtems_timer_ident" );
   printf( "TA1 - timer 2 has id (0x%x)\n", tmid2 );
 
+/* make sure insertion does not unintentionally fire a timer per PR147 */
+
+  TSR_fired = 0;
+
+  puts( "TA1 - rtems_timer_server_fire_after - 1 second" );
+  status = rtems_timer_server_fire_after(
+    tmid, TICKS_PER_SECOND, Should_not_fire_TSR, NULL );
+  directive_failed( status, "rtems_timer_server_fire_after" );
+
+  puts( "TA1 - rtems_task_wake_after - 1/2 second" );
+  status = rtems_task_wake_after( TICKS_PER_SECOND / 2 );
+  directive_failed( status, "rtems_timer_server_fire_after" );
+
+  directive_failed( status, "rtems_timer_server_fire_after" );
+  puts( "TA1 - rtems_timer_server_fire_after - timer 2 in 1/2 second" );
+  status = rtems_timer_server_fire_after(
+    tmid2, TICKS_PER_SECOND / 2, Should_not_fire_TSR, NULL );
+  directive_failed( status, "rtems_timer_server_fire_after" );
+
+  if ( TSR_fired ) {
+    puts( "TA1 - TSR fired and should not have!" );
+    exit(1); 
+  }
+
+  puts( "TA1 - rtems_timer_cancel - timer 1" );
+  status = rtems_timer_cancel( tmid );
+  directive_failed( status, "rtems_timer_cancel" );
+
+  puts( "TA1 - rtems_timer_cancel - timer 2" );
+  status = rtems_timer_cancel( tmid2 );
+  directive_failed( status, "rtems_timer_cancel" );
+
+  
 /* now check that rescheduling an active timer works OK. */
   puts( "TA1 - rtems_timer_server_fire_after - timer 1 in 30 seconds" );
   status = rtems_timer_server_fire_after(