From 29ea901b243265b209271775540d22701f337d57 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 28 Dec 2010 20:29:26 +0100 Subject: [PATCH] Fix the async free() and disconnect() functions To make sure that these functions can also be called from functions other than command callbacks, the flag IN_CALLBACK is introduced that holds whether the context is currently executing a callback. If so, redisAsyncFree() and redisAsyncDisconnect() should delegate their task to the reply processor to avoid segfaults. --- async.c | 46 ++++++++++++++++++++++++++-------------------- hiredis.h | 3 +++ 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/async.c b/async.c index 8068056..e8f7931 100644 --- a/async.c +++ b/async.c @@ -169,33 +169,19 @@ static void __redisAsyncFree(redisAsyncContext *ac) { redisFree(c); } -/* Free's the async context. When REDIS_CONNECTED is set, it could only have - * been called from a command callback so we need to let control return to - * redisProcessCallbacks() before free'ing can continue. - * - * When REDIS_CONNECTED is not set, the first write event has not yet fired and - * we can free immediately. */ +/* Free the async context. When this function is called from a callback, + * control needs to be returned to redisProcessCallbacks() before actual + * free'ing. To do so, a flag is set on the context which is picked up by + * redisProcessCallbacks(). Otherwise, the context is immediately free'd. */ void redisAsyncFree(redisAsyncContext *ac) { redisContext *c = &(ac->c); - if (c->flags & REDIS_CONNECTED) { + if (c->flags & REDIS_IN_CALLBACK) { c->flags |= REDIS_FREEING; } else { __redisAsyncFree(ac); } } -/* Tries to do a clean disconnect from Redis, meaning it stops new commands - * from being issued, but tries to flush the output buffer and execute - * callbacks for all remaining replies. - * - * This functions is generally called from within a callback, so the - * processCallbacks function will pick up the flag when there are no - * more replies. */ -void redisAsyncDisconnect(redisAsyncContext *ac) { - redisContext *c = &(ac->c); - c->flags |= REDIS_DISCONNECTING; -} - /* Helper function to make the disconnect happen and clean up. */ static void __redisAsyncDisconnect(redisAsyncContext *ac) { redisContext *c = &(ac->c); @@ -214,14 +200,32 @@ static void __redisAsyncDisconnect(redisAsyncContext *ac) { /* Execute pending callbacks with NULL reply. */ while (__redisShiftCallback(&ac->replies,&cb) == REDIS_OK) { - if (cb.fn != NULL) + if (cb.fn != NULL) { + c->flags |= REDIS_IN_CALLBACK; cb.fn(ac,NULL,cb.privdata); + c->flags &= ~REDIS_IN_CALLBACK; + } } } __redisAsyncFree(ac); } +/* Tries to do a clean disconnect from Redis, meaning it stops new commands + * from being issued, but tries to flush the output buffer and execute + * callbacks for all remaining replies. When this function is called from a + * callback, there might be more replies and we can safely defer disconnecting + * to redisProcessCallbacks(). Otherwise, we can only disconnect immediately + * when there are no pending callbacks. */ +void redisAsyncDisconnect(redisAsyncContext *ac) { + redisContext *c = &(ac->c); + if (c->flags & REDIS_IN_CALLBACK || ac->replies.head != NULL) { + c->flags |= REDIS_DISCONNECTING; + } else { + __redisAsyncDisconnect(ac); + } +} + void redisProcessCallbacks(redisAsyncContext *ac) { redisContext *c = &(ac->c); redisCallback cb; @@ -245,7 +249,9 @@ void redisProcessCallbacks(redisAsyncContext *ac) { /* Shift callback and execute it */ assert(__redisShiftCallback(&ac->replies,&cb) == REDIS_OK); if (cb.fn != NULL) { + c->flags |= REDIS_IN_CALLBACK; cb.fn(ac,reply,cb.privdata); + c->flags &= ~REDIS_IN_CALLBACK; /* Proceed with free'ing when redisAsyncFree() was called. */ if (c->flags & REDIS_FREEING) { diff --git a/hiredis.h b/hiredis.h index 4e6e8a3..4635280 100644 --- a/hiredis.h +++ b/hiredis.h @@ -68,6 +68,9 @@ * up as soon as possible. */ #define REDIS_FREEING 0x8 +/* Flag that is set when an async callback is executed. */ +#define REDIS_IN_CALLBACK 0x10 + #define REDIS_REPLY_STRING 1 #define REDIS_REPLY_ARRAY 2 #define REDIS_REPLY_INTEGER 3