From 0039fc839131ce059cd08401c1751913d6293098 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 21 Oct 2008 13:33:52 +0200 Subject: bugfix: (potentially big) memory leak on HUP - if queues could not be drained before timeout - thanks to David Lang for pointing this out - added link to german-language forum to doc set --- ChangeLog | 2 ++ doc/manual.html | 13 ++++++------- runtime/queue.c | 45 ++++++++++++++++++++++++++++++++------------- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4ddca1c..876c6d2b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,8 @@ Version 3.19.12 [BETA] (rgerhards), 2008-10-16 now is). - increased maximum size of a configuration statement to 4K (was 1K) - imported all fixes from the stable branch (quite a lot) +- bugfix: (potentially big) memory leak on HUP if queues could not be + drained before timeout - thanks to David Lang for pointing this out --------------------------------------------------------------------------- Version 3.19.11 [BETA] (rgerhards), 2008-08-25 This is a refresh of the beta. No beta-specific fixes have been added. diff --git a/doc/manual.html b/doc/manual.html index 4490f653..f1e2640b 100644 --- a/doc/manual.html +++ b/doc/manual.html @@ -84,13 +84,12 @@ wiki, a community resource which includes rsyslog online documentation (most current version only) -

deutsches rsyslog forum (forum in German language)

And don't forget about the rsyslog mailing list. If you are interested in the "backstage", you diff --git a/runtime/queue.c b/runtime/queue.c index 0768cd77..9f9943bc 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -87,6 +87,30 @@ ENDfunc return pThis->iQueueSize + pThis->iUngottenObjs; } + +/* This function drains the queue in cases where this needs to be done. The most probable + * reason is a HUP which needs to discard data (because the queue is configured to be lossy). + * During a shutdown, this is typically not needed, as the OS frees up ressources and does + * this much quicker than when we clean up ourselvs. -- rgerhards, 2008-10-21 + * This function returns void, as it makes no sense to communicate an error back, even if + * it happens. + */ +static inline void queueDrain(queue_t *pThis) +{ + void *pUsr; + + ASSERT(pThis != NULL); + + /* iQueueSize is not decremented by qDel(), so we need to do it ourselves */ + while(pThis->iQueueSize-- > 0) { + pThis->qDel(pThis, &pUsr); + if(pUsr != NULL) { + objDestruct(pUsr); + } + } +} + + /* --------------- code for disk-assisted (DA) queue modes -------------------- */ @@ -195,14 +219,6 @@ queueTurnOffDAMode(queue_t *pThis) } } - /* TODO: we have a *really biiiiig* memory leak here: if the queue could not be persisted, all of - * its data elements are still in memory. That doesn't really matter if we are terminated, but on - * HUP this memory leaks. We MUST add a loop of destructor calls here. However, this takes time - * (possibly a lot), so it is probably best to have a config variable for that. - * Something for 3.11.1! - * rgerhards, 2008-01-30 - */ - RETiRet; } @@ -460,12 +476,15 @@ static rsRetVal qDestructFixedArray(queue_t *pThis) ASSERT(pThis != NULL); + queueDrain(pThis); /* discard any remaining queue entries */ + if(pThis->tVars.farray.pBuf != NULL) free(pThis->tVars.farray.pBuf); RETiRet; } + static rsRetVal qAddFixedArray(queue_t *pThis, void* in) { DEFiRet; @@ -569,11 +588,11 @@ static rsRetVal qConstructLinkedList(queue_t *pThis) static rsRetVal qDestructLinkedList(queue_t __attribute__((unused)) *pThis) { DEFiRet; - - /* with the linked list type, there is nothing to do here. The - * reason is that the Destructor is only called after all entries - * have bene taken off the queue. In this case, there is nothing - * dynamic left with the linked list. + + queueDrain(pThis); /* discard any remaining queue entries */ + + /* with the linked list type, there is nothing left to do here. The + * reason is that there are no dynamic elements for the list itself. */ RETiRet; -- cgit