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.
This commit is contained in:
antirez 2016-01-08 15:05:14 +01:00
parent 28c291c55c
commit 67b70a1813
4 changed files with 28 additions and 30 deletions

View File

@ -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;
}

View File

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

View File

@ -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

View File

@ -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