summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRainer Gerhards <rgerhards@adiscon.com>2008-02-27 14:12:26 +0000
committerRainer Gerhards <rgerhards@adiscon.com>2008-02-27 14:12:26 +0000
commit58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c (patch)
tree115f7e47fee80a41054b81dd361e92ad224336cf
parent42f8e09d07e9a384d17a675a20117677600eb0f5 (diff)
downloadrsyslog-58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c.tar.gz
rsyslog-58bb094fbaf6fac3ccd4db802db1cfcb37f3d01c.tar.xz
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--ChangeLog5
-rw-r--r--debug.c2
-rw-r--r--queue.c18
-rw-r--r--wtp.c2
4 files changed, 23 insertions, 4 deletions
diff --git a/ChangeLog b/ChangeLog
index addfadb1..cc39a39c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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)
diff --git a/debug.c b/debug.c
index 5814509e..74a5d9dd 100644
--- a/debug.c
+++ b/debug.c
@@ -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? */
diff --git a/queue.c b/queue.c
index fa896437..25261751 100644
--- a/queue.c
+++ b/queue.c
@@ -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);
diff --git a/wtp.c b/wtp.c
index 599e63c3..7e920631 100644
--- a/wtp.c
+++ b/wtp.c
@@ -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]);