From d302fb7571dde5a929c5e80c2bdf4b2133b4f249 Mon Sep 17 00:00:00 2001 From: Andre Lorbach Date: Tue, 16 Jul 2013 09:13:35 +0200 Subject: Fixed return state handling in ConsumerDA The queue full loop fix added a problem to the queue when rsyslog was shutdown. This problem has been corrected now. Conflicts: runtime/queue.c --- runtime/queue.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'runtime/queue.c') diff --git a/runtime/queue.c b/runtime/queue.c index d5ad37a1..5652efe2 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -1930,8 +1930,25 @@ ConsumerDA(qqueue_t *pThis, wti_t *pWti) pthread_setcancelstate(iCancelStateSave, NULL); finalize_it: - if(iRet != RS_RET_OK && iRet != RS_RET_ERR_QUEUE_EMERGENCY) { - iRet = RS_RET_OK; + /* Check the last return state of qqueueEnqMsg. If an error was returned, we acknowledge it only. + * Unless the error code is RS_RET_ERR_QUEUE_EMERGENCY, we resett the return state to RS_RET_OK. + * Otherwise the Caller functions would run into an infinite Loop trying to enqueue the + * same messages over and over again. + * + * However we do NOT overwrite positive return states like + * RS_RET_TERMINATE_NOW, + * RS_RET_NO_RUN, + * RS_RET_IDLE, + * RS_RET_TERMINATE_WHEN_IDLE + * These return states are important for Queue handling of the upper laying functions. + */ + if( iRet != RS_RET_OK && + iRet != RS_RET_ERR_QUEUE_EMERGENCY && + iRet < 0) { + DBGOPRINT((obj_t*) pThis, "ConsumerDA:qqueueEnqMsg Resetting iRet from %d back to RS_RET_OK\n", iRet); + iRet = RS_RET_OK; + } else { + DBGOPRINT((obj_t*) pThis, "ConsumerDA:qqueueEnqMsg returns with iRet %d\n", iRet); } /* now we are done, but potentially need to re-aquire the mutex */ -- cgit v1.2.3 From 1c6449022c423c13a8828028cb68afe30e3c2f46 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 16 Jul 2013 10:55:39 +0200 Subject: add note on a potential future troublespot --- runtime/queue.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'runtime/queue.c') diff --git a/runtime/queue.c b/runtime/queue.c index 5652efe2..5ccdd814 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -1931,7 +1931,7 @@ ConsumerDA(qqueue_t *pThis, wti_t *pWti) finalize_it: /* Check the last return state of qqueueEnqMsg. If an error was returned, we acknowledge it only. - * Unless the error code is RS_RET_ERR_QUEUE_EMERGENCY, we resett the return state to RS_RET_OK. + * Unless the error code is RS_RET_ERR_QUEUE_EMERGENCY, we reset the return state to RS_RET_OK. * Otherwise the Caller functions would run into an infinite Loop trying to enqueue the * same messages over and over again. * @@ -1941,6 +1941,13 @@ finalize_it: * RS_RET_IDLE, * RS_RET_TERMINATE_WHEN_IDLE * These return states are important for Queue handling of the upper laying functions. + * RGer: Note that checking for iRet < 0 is a bit bold. In theory, positive iRet + * values are "OK" states, and things that the caller shall deal with. However, + * this has not been done so consistently. Andre convinced me that the current + * code is an elegant solution. However, if problems with queue workers and/or + * shutdown come up, this code here should be looked at suspiciously. In those + * cases it may work out to check all status codes explicitely, just to avoid + * a pitfall due to unexpected states being passed on to the caller. */ if( iRet != RS_RET_OK && iRet != RS_RET_ERR_QUEUE_EMERGENCY && -- cgit v1.2.3