diff options
author | Rainer Gerhards <rgerhards@adiscon.com> | 2008-02-27 14:12:26 +0000 |
---|---|---|
committer | Rainer Gerhards <rgerhards@adiscon.com> | 2008-02-27 14:12:26 +0000 |
commit | 58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c (patch) | |
tree | 115f7e47fee80a41054b81dd361e92ad224336cf | |
parent | 42f8e09d07e9a384d17a675a20117677600eb0f5 (diff) | |
download | rsyslog-58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c.tar.gz rsyslog-58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c.tar.bz2 rsyslog-58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c.zip |
bugfix: during queue shutdown, an assert invalidly triggered when the
primary queue's DA worker was terminated while the DA queue's regular
worker was still executing. This could result in a segfault during
shutdown. tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=41
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | debug.c | 2 | ||||
-rw-r--r-- | queue.c | 18 | ||||
-rw-r--r-- | wtp.c | 2 |
4 files changed, 23 insertions, 4 deletions
@@ -17,6 +17,11 @@ Version 3.11.6 (rgerhards), 2008-02-?? which could cause several problems to the engine (unpredictable results). This situation should have happened only in very rare cases. tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=40 +- bugfix: during queue shutdown, an assert invalidly triggered when + the primary queue's DA worker was terminated while the DA queue's + regular worker was still executing. This could result in a segfault + during shutdown. + tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=41 - bugfix: imfile could abort under extreme stress conditions (when it was terminated before it could open all of its to be monitored files) @@ -57,7 +57,7 @@ static dbgThrdInfo_t *dbgGetThrdInfo(void); /* static data (some time to be replaced) */ int Debug; /* debug flag - read-only after startup */ int debugging_on = 0; /* read-only, except on sig USR1 */ -static int bLogFuncFlow = 1; /* shall the function entry and exit be logged to the debug log? */ +static int bLogFuncFlow = 0; /* shall the function entry and exit be logged to the debug log? */ static int bLogAllocFree = 0; /* shall calls to (m/c)alloc and free be logged to the debug log? */ static int bPrintFuncDBOnExit = 0; /* shall the function entry and exit be logged to the debug log? */ static int bPrintMutexAction = 0; /* shall mutex calls be printed to the debug log? */ @@ -1549,6 +1549,13 @@ queueIsIdleReg(queue_t *pThis) * we are the DA (child) queue, that means the DA queue is empty. In that case, we * need to signal the parent queue's DA worker, so that it can terminate DA mode. * rgerhards, 2008-01-26 + * rgerhards, 2008-02-27: HOWEVER, in a shutdown condition, it may be that the parent's worker thread pool + * has already been terminated and destructed. This *is* a legal condition and happens + * from time to time in practice. So we need to signal only if there still is a + * parent DA worker queue. Please keep in mind that the the parent's DA worker + * pool is DIFFERENT from our (DA queue) regular worker pool. So when the parent's + * pWtpDA is destructed, there can still be some of our (DAq/wtp) threads be running. + * I am telling this, because I, too, always get confused by those... */ static rsRetVal queueRegOnWrkrShutdown(queue_t *pThis) @@ -1558,9 +1565,10 @@ queueRegOnWrkrShutdown(queue_t *pThis) ISOBJ_TYPE_assert(pThis, queue); if(pThis->pqParent != NULL) { - ASSERT(pThis->pqParent->pWtpDA != NULL); pThis->pqParent->bChildIsDone = 1; /* indicate we are done */ - wtpAdviseMaxWorkers(pThis->pqParent->pWtpDA, 1); /* reactivate DA worker (always 1) */ + if(pThis->pqParent->pWtpDA != NULL) { /* see comment in function header from 2008-02-27 */ + wtpAdviseMaxWorkers(pThis->pqParent->pWtpDA, 1); /* reactivate DA worker (always 1) */ + } } RETiRet; @@ -1824,10 +1832,14 @@ CODESTARTobjDestruct(queue) * Note that the wtp must be destructed first, it may be in cancel cleanup handler * *right now* and actually *need* to access the queue object to persist some final * data (re-queueing case). So we need to destruct the wtp first, which will make - * sure all workers have terminated. + * sure all workers have terminated. Please note that this also generates a situation + * where it is possible that the DA queue has a parent pointer but the parent has + * no WtpDA associated with it - which is perfectly legal thanks to this code here. */ if(pThis->pWtpDA != NULL) { +RUNLOG_STR("wtpDA is being destructed\n"); wtpDestruct(&pThis->pWtpDA); +RUNLOG_STR("wtpDA is being destructed - done\n"); } if(pThis->pqDA != NULL) { queueDestruct(&pThis->pqDA); @@ -128,6 +128,8 @@ finalize_it: BEGINobjDestruct(wtp) /* be sure to specify the object type also in END and CODESTART macros! */ int i; CODESTARTobjDestruct(wtp) + wtpProcessThrdChanges(pThis); /* process thread changes one last time */ + /* destruct workers */ for(i = 0 ; i < pThis->iNumWorkerThreads ; ++i) wtiDestruct(&pThis->pWrkr[i]); |