提交 8ac7af1c 编写于 作者: A antirez

Fix ae.c when a timer finalizerProc adds an event.

While this feature is not used by Redis, ae.c implements the ability for
a timer to call a finalizer callback when an timer event is deleted.
This feature was bugged since the start, and because it was never used
we never noticed a problem. However Anthony LaTorre was using the same
library in order to implement a different system: he found a bug that he
describes as follows, and which he fixed with the patch in this commit,
sent me by private email:

    --- Anthony email ---

've found one bug in the current implementation of the timed events.
It's possible to lose track of a timed event if an event is added in
the finalizerProc of another event.

For example, suppose you start off with three timed events 1, 2, and
3. Then the linked list looks like:

3 -> 2 -> 1

Then, you run processTimeEvents and events 2 and 3 finish, so now the
list looks like:

-1 -> -1 -> 2

Now, on the next iteration of processTimeEvents it starts by deleting
the first event, and suppose this finalizerProc creates a new event,
so that the list looks like this:

4 -> -1 -> 2

On the next iteration of the while loop, when it gets to the second
event, the variable prev is still set to NULL, so that the head of the
event loop after the next event will be set to 2, i.e. after deleting
the next event the event loop will look like:

2

and the event with id 4 will be lost.

I've attached an example program to illustrate the issue. If you run
it you will see that it prints:

```
foo id = 0
spam!
```

But if you uncomment line 29 and run it again it won't print "spam!".

    --- End of email ---

Test.c source code is as follows:

    #include "ae.h"
    #include <stdio.h>

    aeEventLoop *el;

    int foo(struct aeEventLoop *el, long long id, void *data)
    {
	printf("foo id = %lld\n", id);

	return AE_NOMORE;
    }

    int spam(struct aeEventLoop *el, long long id, void *data)
    {
	printf("spam!\n");

	return AE_NOMORE;
    }

    void bar(struct aeEventLoop *el, void *data)
    {
	aeCreateTimeEvent(el, 0, spam, NULL, NULL);
    }

    int main(int argc, char **argv)
    {
	el = aeCreateEventLoop(100);

	//aeCreateTimeEvent(el, 0, foo, NULL, NULL);
	aeCreateTimeEvent(el, 0, foo, NULL, bar);

	aeMain(el);

	return 0;
    }

Anthony fixed the problem by using a linked list for the list of timers, and
sent me back this patch after he tested the code in production for some time.
The code looks sane to me, so committing it to Redis.
上级 674909f4
......@@ -219,7 +219,10 @@ long long aeCreateTimeEvent(aeEventLoop *eventLoop, long long milliseconds,
te->timeProc = proc;
te->finalizerProc = finalizerProc;
te->clientData = clientData;
te->prev = NULL;
te->next = eventLoop->timeEventHead;
if (te->next)
te->next->prev = te;
eventLoop->timeEventHead = te;
return id;
}
......@@ -266,7 +269,7 @@ static aeTimeEvent *aeSearchNearestTimer(aeEventLoop *eventLoop)
/* Process time events */
static int processTimeEvents(aeEventLoop *eventLoop) {
int processed = 0;
aeTimeEvent *te, *prev;
aeTimeEvent *te;
long long maxId;
time_t now = time(NULL);
......@@ -287,7 +290,6 @@ static int processTimeEvents(aeEventLoop *eventLoop) {
}
eventLoop->lastTime = now;
prev = NULL;
te = eventLoop->timeEventHead;
maxId = eventLoop->timeEventNextId-1;
while(te) {
......@@ -297,10 +299,12 @@ static int processTimeEvents(aeEventLoop *eventLoop) {
/* Remove events scheduled for deletion. */
if (te->id == AE_DELETED_EVENT_ID) {
aeTimeEvent *next = te->next;
if (prev == NULL)
eventLoop->timeEventHead = te->next;
if (te->prev)
te->prev->next = te->next;
else
prev->next = te->next;
eventLoop->timeEventHead = te->next;
if (te->next)
te->next->prev = te->prev;
if (te->finalizerProc)
te->finalizerProc(eventLoop, te->clientData);
zfree(te);
......@@ -332,7 +336,6 @@ static int processTimeEvents(aeEventLoop *eventLoop) {
te->id = AE_DELETED_EVENT_ID;
}
}
prev = te;
te = te->next;
}
return processed;
......
......@@ -83,6 +83,7 @@ typedef struct aeTimeEvent {
aeTimeProc *timeProc;
aeEventFinalizerProc *finalizerProc;
void *clientData;
struct aeTimeEvent *prev;
struct aeTimeEvent *next;
} aeTimeEvent;
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册