diff --git a/timing.c b/timing.c index 6dd8aa5e..abb9b466 100644 --- a/timing.c +++ b/timing.c @@ -97,7 +97,14 @@ long schedule_timer(int ticks, timer_fn_t fn, void *ctx) init_timers(); when = ticks + GETTICKCOUNT(); - assert(when - now > 0); + + /* + * Just in case our various defences against timing skew fail + * us: if we try to schedule a timer that's already in the + * past, we instead schedule it for the immediate future. + */ + if (when - now <= 0) + when = now + 1; t = snew(struct timer); t->fn = fn; @@ -133,6 +140,58 @@ int run_timers(long anow, long *next) init_timers(); +#ifdef TIMING_SYNC + /* + * In this ifdef I put some code which deals with the + * possibility that `anow' disagrees with GETTICKCOUNT by a + * significant margin. Our strategy for dealing with it differs + * depending on platform, because on some platforms + * GETTICKCOUNT is more likely to be right whereas on others + * `anow' is a better gold standard. + */ + { + long tnow = GETTICKCOUNT(); + + if (tnow + TICKSPERSEC/50 - anow < 0 || + anow + TICKSPERSEC/50 - tnow < 0 + ) { +#if defined TIMING_SYNC_ANOW + /* + * If anow is accurate and the tick count is wrong, + * this is likely to be because the tick count is + * derived from the system clock which has changed (as + * can occur on Unix). Therefore, we resolve this by + * inventing an offset which is used to adjust all + * future output from GETTICKCOUNT. + * + * A platform which defines TIMING_SYNC_ANOW is + * expected to have also defined this offset variable + * in (its platform-specific adjunct to) putty.h. + * Therefore we can simply reference it here and assume + * that it will exist. + */ + tickcount_offset += anow - tnow; +#elif defined TIMING_SYNC_TICKCOUNT + /* + * If the tick count is more likely to be accurate, we + * simply use that as our time value, which may mean we + * run no timers in this call (because we got called + * early), or alternatively it may mean we run lots of + * timers in a hurry because we were called late. + */ + anow = tnow; +#else +/* + * Any platform which defines TIMING_SYNC must also define one of the two + * auxiliary symbols TIMING_SYNC_ANOW and TIMING_SYNC_TICKCOUNT, to + * indicate which measurement to trust when the two disagree. + */ +#error TIMING_SYNC definition incomplete +#endif + } + } +#endif + now = anow; while (1) { diff --git a/unix/unix.h b/unix/unix.h index cfc48f32..54cf39f2 100644 --- a/unix/unix.h +++ b/unix/unix.h @@ -47,6 +47,11 @@ unsigned long getticks(void); /* based on gettimeofday(2) */ #define GETTICKCOUNT getticks #define TICKSPERSEC 1000 /* we choose to use milliseconds */ #define CURSORBLINK 450 /* no standard way to set this */ +/* getticks() works using gettimeofday(), so it's vulnerable to system clock + * changes causing chaos. Therefore, we provide a compensation mechanism. */ +#define TIMING_SYNC +#define TIMING_SYNC_ANOW +extern long tickcount_offset; #define WCHAR wchar_t #define BYTE unsigned char diff --git a/unix/uxmisc.c b/unix/uxmisc.c index 28ae83a2..c613a204 100644 --- a/unix/uxmisc.c +++ b/unix/uxmisc.c @@ -3,6 +3,7 @@ */ #include +#include #include #include #include @@ -10,6 +11,8 @@ #include "putty.h" +long tickcount_offset = 0; + unsigned long getticks(void) { struct timeval tv; @@ -19,7 +22,7 @@ unsigned long getticks(void) * because we need a decent number of them to fit into a 32-bit * word so it can be used for keepalives. */ - return tv.tv_sec * 1000 + tv.tv_usec / 1000; + return tv.tv_sec * 1000 + tv.tv_usec / 1000 + tickcount_offset; } Filename filename_from_str(const char *str) diff --git a/unix/uxplink.c b/unix/uxplink.c index 6bc1b61b..c0261578 100644 --- a/unix/uxplink.c +++ b/unix/uxplink.c @@ -670,8 +670,27 @@ int main(int argc, char **argv) ret = select(maxfd, &rset, &wset, &xset, ptv); if (ret == 0) now = next; - else - now = GETTICKCOUNT(); + else { + long newnow = GETTICKCOUNT(); + /* + * Check to see whether the system clock has + * changed massively during the select. + */ + if (newnow - now < 0 || newnow - now > next - now) { + /* + * If so, look at the elapsed time in the + * select and use it to compute a new + * tickcount_offset. + */ + long othernow = now + tv.tv_sec * 1000 + tv.tv_usec / 1000; + /* So we'd like GETTICKCOUNT to have returned othernow, + * but instead it return newnow. Hence ... */ + tickcount_offset += othernow - newnow; + now = othernow; + } else { + now = newnow; + } + } } while (ret < 0 && errno == EINTR); if (ret < 0) { diff --git a/unix/uxsftp.c b/unix/uxsftp.c index 7045c322..f5fc12e3 100644 --- a/unix/uxsftp.c +++ b/unix/uxsftp.c @@ -433,8 +433,27 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok) ret = select(maxfd, &rset, &wset, &xset, ptv); if (ret == 0) now = next; - else - now = GETTICKCOUNT(); + else { + long newnow = GETTICKCOUNT(); + /* + * Check to see whether the system clock has + * changed massively during the select. + */ + if (newnow - now < 0 || newnow - now > next - now) { + /* + * If so, look at the elapsed time in the + * select and use it to compute a new + * tickcount_offset. + */ + long othernow = now + tv.tv_sec * 1000 + tv.tv_usec / 1000; + /* So we'd like GETTICKCOUNT to have returned othernow, + * but instead it return newnow. Hence ... */ + tickcount_offset += othernow - newnow; + now = othernow; + } else { + now = newnow; + } + } } while (ret < 0 && errno != EINTR); } while (ret == 0); diff --git a/windows/winstuff.h b/windows/winstuff.h index 28dd08db..177c7d2f 100644 --- a/windows/winstuff.h +++ b/windows/winstuff.h @@ -138,6 +138,15 @@ GLOBAL void *logctx; #define FILTER_WAVE_FILES ("Wave Files (*.wav)\0*.WAV\0" \ "All Files (*.*)\0*\0\0\0") +/* + * On some versions of Windows, it has been known for WM_TIMER to + * occasionally get its callback time simply wrong, and call us + * back several minutes early. Defining these symbols enables + * compensation code in timing.c. + */ +#define TIMING_SYNC +#define TIMING_SYNC_TICKCOUNT + /* * winnet.c dynamically loads WinSock 2 or WinSock 1 depending on * what it can get, which means any WinSock routines used outside