Sebastian Kuschel reports that pfd_closing can be called for a socket
[u/mdw/putty] / timing.c
index 3eb3299..ccd76cd 100644 (file)
--- a/timing.c
+++ b/timing.c
@@ -7,6 +7,24 @@
  * changes; and, very importantly, it tracks the context pointers
  * passed to schedule_timer(), so that if a context is freed all
  * the timers associated with it can be immediately annulled.
+ *
+ *
+ * The problem is that computer clocks aren't perfectly accurate.
+ * The GETTICKCOUNT function returns a 32bit number that normally
+ * increases by about 1000 every second. On windows this uses the PC's
+ * interrupt timer and so is only accurate to around 20ppm.  On unix it's
+ * a value that's calculated from the current UTC time and so is in theory
+ * accurate in the long term but may jitter and jump in the short term.
+ *
+ * What PuTTY needs from these timers is simply a way of delaying the
+ * calling of a function for a little while, if it's occasionally called a
+ * little early or late that's not a problem. So to protect against clock
+ * jumps schedule_timer records the time that it was called in the timer
+ * structure. With this information the run_timers function can see when
+ * the current GETTICKCOUNT value is after the time the event should be
+ * fired OR before the time it was set. In the latter case the clock must
+ * have jumped, the former is (probably) just the normal passage of time.
+ *
  */
 
 #include <assert.h>
 struct timer {
     timer_fn_t fn;
     void *ctx;
-    long now;
+    unsigned long now;
+    unsigned long when_set;
 };
 
 static tree234 *timers = NULL;
-static long now = 0L;
+static tree234 *timer_contexts = NULL;
+static unsigned long now = 0L;
 
 static int compare_timers(void *av, void *bv)
 {
@@ -40,14 +60,12 @@ static int compare_timers(void *av, void *bv)
      * Failing that, compare on the other two fields, just so that
      * we don't get unwanted equality.
      */
-#ifdef __LCC__
+#if defined(__LCC__) || defined(__clang__)
     /* lcc won't let us compare function pointers. Legal, but annoying. */
     {
        int c = memcmp(&a->fn, &b->fn, sizeof(a->fn));
-       if (c < 0)
-           return -1;
-       else if (c > 0)
-           return +1;
+       if (c)
+           return c;
     }
 #else    
     if (a->fn < b->fn)
@@ -70,10 +88,8 @@ static int compare_timers(void *av, void *bv)
 
 static int compare_timer_contexts(void *av, void *bv)
 {
-    struct timer *at = (struct timer *)av;
-    struct timer *bt = (struct timer *)bv;
-    char *a = (char *)at->ctx;
-    char *b = (char *)bt->ctx;
+    char *a = (char *)av;
+    char *b = (char *)bv;
     if (a < b)
        return -1;
     else if (a > b)
@@ -85,18 +101,20 @@ static void init_timers(void)
 {
     if (!timers) {
        timers = newtree234(compare_timers);
+       timer_contexts = newtree234(compare_timer_contexts);
        now = GETTICKCOUNT();
     }
 }
 
-long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
+unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
 {
-    long when;
+    unsigned long when;
     struct timer *t, *first;
 
     init_timers();
 
-    when = ticks + GETTICKCOUNT();
+    now = GETTICKCOUNT();
+    when = ticks + now;
 
     /*
      * Just in case our various defences against timing skew fail
@@ -110,9 +128,12 @@ long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
     t->fn = fn;
     t->ctx = ctx;
     t->now = when;
+    t->when_set = now;
 
     if (t != add234(timers, t)) {
        sfree(t);                      /* identical timer already exists */
+    } else {
+       add234(timer_contexts, t->ctx);/* don't care if this fails */
     }
 
     first = (struct timer *)index234(timers, 0);
@@ -132,65 +153,13 @@ long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
  * Returns the time (in ticks) expected until the next timer after
  * that triggers.
  */
-int run_timers(long anow, long *next)
+int run_timers(unsigned long anow, unsigned long *next)
 {
     struct timer *first;
 
     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;
+    now = GETTICKCOUNT();
 
     while (1) {
        first = (struct timer *)index234(timers, 0);
@@ -198,7 +167,15 @@ int run_timers(long anow, long *next)
        if (!first)
            return FALSE;              /* no timers remaining */
 
-       if (first->now - now <= 0) {
+       if (find234(timer_contexts, first->ctx, NULL) == NULL) {
+           /*
+            * This timer belongs to a context that has been
+            * expired. Delete it without running.
+            */
+           delpos234(timers, 0);
+           sfree(first);
+       } else if (now - (first->when_set - 10) >
+                  first->now - (first->when_set - 10)) {
            /*
             * This timer is active and has reached its running
             * time. Run it.
@@ -222,24 +199,13 @@ int run_timers(long anow, long *next)
  */
 void expire_timer_context(void *ctx)
 {
-    struct timer *ptr;
-    struct timer exemplar;
-
-    if (!timers) return;
-
-    exemplar.ctx = ctx;
-    /* don't care about initialisation of other members */
-
-    /* Dispose of all timers with this context */
-    while ((ptr = (struct timer *)find234(timers, &exemplar,
-                                         compare_timer_contexts))) {
-       del234(timers, ptr);
-       sfree(ptr);
-    }
+    init_timers();
 
-    /* Dispose of timer tree itself if none are left */
-    if (count234(timers) == 0) {
-       freetree234(timers);
-       timers = NULL;
-    }
+    /*
+     * We don't bother to check the return value; if the context
+     * already wasn't in the tree (presumably because no timers
+     * ever actually got scheduled for it) then that's fine and we
+     * simply don't need to do anything.
+     */
+    del234(timer_contexts, ctx);
 }