WIN32 osdtime should handle the system time change properly

Bug #697517 reported by Alex Chen
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
EPICS Base
Fix Released
Medium
Unassigned
3.14
Fix Released
Medium
Jeff Hill

Bug Description

EPICS Base 3.14.9

File: \src\libCom\osi\os\WIN32\osdTime.cpp

Line 466: ' if ( fileTimeDiff == 0 ) {'

Problem:
If user changes the system at some point, the 'fileTimeDiff' here could be negative here.

Suggestion:
We could simply change ' if ( fileTimeDiff == 0 ) {' to ' if ( fileTimeDiff <= 0 ) {'

Revision history for this message
Jeff Hill (johill-lanl) wrote :

> We could simply change ' if ( fileTimeDiff == 0 ) {' to ' if ( fileTimeDiff <= 0 ) {'

This is better, but maybe not enough because the PLL will stop functioning if they move the time back

Changed in epics-base:
status: New → Confirmed
Revision history for this message
Jeff Hill (johill-lanl) wrote :

Actually your suggestion is ok I see now because of prev line says "this->lastFileTimePLL = curFileTime.QuadPart;"

Revision history for this message
Jeff Hill (johill-lanl) wrote :

Committed this fix to the R3.14 branch

=== modified file 'src/libCom/osi/os/WIN32/osdTime.cpp'
--- src/libCom/osi/os/WIN32/osdTime.cpp 2011-11-10 21:52:22 +0000
+++ src/libCom/osi/os/WIN32/osdTime.cpp 2011-11-11 00:16:16 +0000
@@ -82,7 +82,7 @@
     epicsTimerQueueActive * pTimerQueue;
     epicsTimer * pTimer;
     bool perfCtrPresent;
-
+ static const int pllDelay; /* integer seconds */
     epicsTimerNotify::expireStatus expire ( const epicsTime & );
 };

@@ -92,6 +92,8 @@
 static const LONGLONG ET_TICKS_PER_FT_TICK =
             EPICS_TIME_TICKS_PER_SEC / FILE_TIME_TICKS_PER_SEC;

+const int currentTime :: pllDelay = 5;
+
 //
 // Start and register time provider
 //
@@ -433,10 +435,10 @@
     this->lastFileTimePLL = curFileTime.QuadPart;

     // discard glitches
- if ( fileTimeDiff == 0 ) {
+ if ( fileTimeDiff <= 0 ) {
         LeaveCriticalSection( & this->mutex );
- debugPrintf ( ( "currentTime: file time difference in PLL was zero\n" ) );
- return expireStatus ( restart, 1.0 /* sec */ );
+ debugPrintf ( ( "currentTime: file time difference in PLL was less than zero\n" ) );
+ return expireStatus ( restart, pllDelay /* sec */ );
     }

     LONGLONG freq = ( FILE_TIME_TICKS_PER_SEC * perfCounterDiff ) / fileTimeDiff;
@@ -450,7 +452,7 @@
             static_cast < int > ( -bound ),
             static_cast < int > ( delta ),
             static_cast < int > ( bound ) ) );
- return expireStatus ( restart, 1.0 /* sec */ );
+ return expireStatus ( restart, pllDelay /* sec */ );
     }

     // update feedback loop estimating the performance counter's frequency
@@ -476,6 +478,20 @@
             ( MAXLONGLONG - this->lastPerfCounter )
                             + ( curPerfCounter.QuadPart - MINLONGLONG ) + 1;
     }
+
+ // discard performance counter delay measurement glitches
+ {
+ const LONGLONG expectedDly = this->perfCounterFreq * pllDelay;
+ const LONGLONG bnd = expectedDly / 4;
+ if ( perfCounterDiffSinceLastFetch <= 0 ||
+ perfCounterDiffSinceLastFetch >= expectedDly + bnd ) {
+ LeaveCriticalSection( & this->mutex );
+ debugPrintf ( ( "perf ctr measured delay out of bounds m=%d max=%d\n",
+ static_cast < int > ( perfCounterDiffSinceLastFetch ),
+ static_cast < int > ( expectedDly + bnd ) ) );
+ return expireStatus ( restart, pllDelay /* sec */ );
+ }
+ }

     // Update the current estimated time.
     this->epicsTimeLast +=
@@ -561,7 +577,7 @@

     LeaveCriticalSection ( & this->mutex );

- return expireStatus ( restart, 1.0 /* sec */ );
+ return expireStatus ( restart, pllDelay /* sec */ );
 }

 void currentTime::startPLL ()
@@ -570,7 +586,7 @@
     if ( this->perfCtrPresent && ! this->pTimerQueue ) {
         this->pTimerQueue = & epicsTimerQueueActive::allocate ( true );
         this->pTimer = & this->pTimerQueue->createTimer ();
- this->pTimer->start ( *this, 1.0 );
+ this->pTimer->start ( *this, pllDelay );
     }
 }

Andrew Johnson (anj)
no longer affects: epics-base/3.15
Andrew Johnson (anj)
Changed in epics-base:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.