Render timing.c robust in the face of strangeness. The strangenesses

in question vary per OS: on Windows the problem is that WM_TIMER
sometimes goes off too early, so that GetTickCount() is right and
the callback time is wrong, whereas on Unix the problem is that my
GETTICKCOUNT implementation comes from the system clock which means
it can change suddenly and non-monotonically if the sysadmin is
messing about (meaning that the timing of callbacks from GTK or
select timeouts is _more_ likely to be right than GETTICKCOUNT).
This checkin provides band-aid workarounds for both problems, which
aren't pretty but ought to at least prevent catastrophic assertion
failure.

[originally from svn r5556]

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) {

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

unix/uxmisc.c

@@ -3,6 +3,7 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <unistd.h>
 #include <sys/time.h>
 #include <sys/types.h>
@@ -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)

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) {

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);
 

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