Fix ae.c to avoid timers infinite loop.

This fix was suggested by Anthony LaTorre, that provided also a good
test case that was used to verify the fix.

The problem with the old implementation is that, the time returned by
a timer event (that is the time after it want to run again) is added
to the event *start time*. So if the event takes, in order to run, more
than the time it says it want to be scheduled again for running, an
infinite loop is triggered.

src/ae.c

@@ -221,21 +221,12 @@ long long aeCreateTimeEvent(aeEventLoop *eventLoop, long long milliseconds,
 
 int aeDeleteTimeEvent(aeEventLoop *eventLoop, long long id)
 {
-    aeTimeEvent *te, *prev = NULL;
-
-    te = eventLoop->timeEventHead;
+    aeTimeEvent *te = eventLoop->timeEventHead;
     while(te) {
         if (te->id == id) {
-            if (prev == NULL)
-                eventLoop->timeEventHead = te->next;
-            else
-                prev->next = te->next;
-            if (te->finalizerProc)
-                te->finalizerProc(eventLoop, te->clientData);
-            zfree(te);
+            te->id = AE_DELETED_EVENT_ID;
             return AE_OK;
         }
-        prev = te;
         te = te->next;
     }
     return AE_ERR; /* NO event with the specified ID found */
@@ -270,7 +261,7 @@ static aeTimeEvent *aeSearchNearestTimer(aeEventLoop *eventLoop)
 /* Process time events */
 static int processTimeEvents(aeEventLoop *eventLoop) {
     int processed = 0;
-    aeTimeEvent *te;
+    aeTimeEvent *te, *prev;
     long long maxId;
     time_t now = time(NULL);
 
@@ -291,12 +282,28 @@ static int processTimeEvents(aeEventLoop *eventLoop) {
     }
     eventLoop->lastTime = now;
 
+    prev = NULL;
     te = eventLoop->timeEventHead;
     maxId = eventLoop->timeEventNextId-1;
     while(te) {
         long now_sec, now_ms;
         long long id;
 
+        /* Remove events scheduled for deletion. */
+        if (te->id == AE_DELETED_EVENT_ID) {
+            aeTimeEvent *next = te->next;
+            if (prev == NULL)
+                eventLoop->timeEventHead = te->next;
+            else
+                prev->next = te->next;
+            if (te->finalizerProc)
+                te->finalizerProc(eventLoop, te->clientData);
+            zfree(te);
+            te = next;
+            continue;
+        }
+
+        /* Don't process time events created by time events in this iteration. */
         if (te->id > maxId) {
             te = te->next;
             continue;
@@ -310,28 +317,14 @@ static int processTimeEvents(aeEventLoop *eventLoop) {
             id = te->id;
             retval = te->timeProc(eventLoop, id, te->clientData);
             processed++;
-            /* After an event is processed our time event list may
-             * no longer be the same, so we restart from head.
-             * Still we make sure to don't process events registered
-             * by event handlers itself in order to don't loop forever.
-             * To do so we saved the max ID we want to handle.
-             *
-             * FUTURE OPTIMIZATIONS:
-             * Note that this is NOT great algorithmically. Redis uses
-             * a single time event so it's not a problem but the right
-             * way to do this is to add the new elements on head, and
-             * to flag deleted elements in a special way for later
-             * deletion (putting references to the nodes to delete into
-             * another linked list). */
             if (retval != AE_NOMORE) {
                 aeAddMillisecondsToNow(retval,&te->when_sec,&te->when_ms);
             } else {
-                aeDeleteTimeEvent(eventLoop, id);
+                te->id = AE_DELETED_EVENT_ID;
             }
-            te = eventLoop->timeEventHead;
-        } else {
-            te = te->next;
         }
+        prev = te;
+        te = te->next;
     }
     return processed;
 }

src/ae.h

@@ -33,6 +33,8 @@
 #ifndef __AE_H__
 #define __AE_H__
 
+#include <time.h>
+
 #define AE_OK 0
 #define AE_ERR -1
 
@@ -46,6 +48,7 @@
 #define AE_DONT_WAIT 4
 
 #define AE_NOMORE -1
+#define AE_DELETED_EVENT_ID -1
 
 /* Macros */
 #define AE_NOTUSED(V) ((void) V)

src/anet.h

@@ -31,6 +31,8 @@
 #ifndef ANET_H
 #define ANET_H
 
+#include <sys/types.h>
+
 #define ANET_OK 0
 #define ANET_ERR -1
 #define ANET_ERR_LEN 256

src/redis-trib.rb

@@ -941,7 +941,7 @@ class RedisTrib
                     source.r.client.call(["migrate",target.info[:host],target.info[:port],"",0,@timeout,:replace,:keys,*keys])
                 else
                     puts ""
-                    xputs "[ERR] #{e}"
+                    xputs "[ERR] Calling MIGRATE: #{e}"
                     exit 1
                 end
             end