WIN32 osdtime should handle the system time change properly

Bug #697517 reported by Alex Chen on 2011-01-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
EPICS Base
Medium
Unassigned
3.14
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 ) {'

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
Jeff Hill (johill-lanl) wrote :

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

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) on 2011-12-12
no longer affects: epics-base/3.15
Andrew Johnson (anj) on 2011-12-12
Changed in epics-base:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers