From b61be5dccaacfdfa7878e5e497022e073081f5f8 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 16 Dec 2009 15:30:45 +0100 Subject: bugfix: error during beginTransaction() was not properly handled Suspension during beginTransaction() did not properly cause the action to be suspended, thus we entered an endless loop. --- action.c | 22 ++++++++++++++++++++-- doc/action_state.dot | 1 + 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/action.c b/action.c index 9178b991..5724de2f 100644 --- a/action.c +++ b/action.c @@ -491,14 +491,17 @@ static rsRetVal actionDoRetry(action_t *pThis, time_t ttNow) ASSERT(pThis != NULL); +dbgprintf("XXX: we are doing a retry, action state %d\n", pThis->eState); iRetries = 0; while(pThis->eState == ACT_STATE_RTRY) { iRet = pThis->pMod->tryResume(pThis->pModData); +dbgprintf("XXX: result of tryResume %d\n", iRet); if(iRet == RS_RET_OK) { actionSetState(pThis, ACT_STATE_RDY); } else if(iRet == RS_RET_SUSPENDED) { /* max retries reached? */ if((pThis->iResumeRetryCount != -1 && iRetries >= pThis->iResumeRetryCount)) { +dbgprintf("XXX: action suspeneded!\n"); actionSuspend(pThis, ttNow); } else { ++pThis->iNbrResRtry; @@ -530,6 +533,7 @@ static rsRetVal actionTryResume(action_t *pThis) ASSERT(pThis != NULL); +dbgprintf("XXX: actionTryResume, action state %d\n", pThis->eState); if(pThis->eState == ACT_STATE_SUSP) { /* if we are suspended, we need to check if the timeout expired. * for this handling, we must always obtain a fresh timestamp. We used @@ -575,8 +579,20 @@ static rsRetVal actionPrepare(action_t *pThis) * action state accordingly */ if(pThis->eState == ACT_STATE_RDY) { - CHKiRet(pThis->pMod->mod.om.beginTransaction(pThis->pModData)); - actionSetState(pThis, ACT_STATE_ITX); + iRet = pThis->pMod->mod.om.beginTransaction(pThis->pModData); +dbgprintf("XXX: actionPrepare, result of beginTranscation %d\n", iRet); + switch(iRet) { + case RS_RET_OK: + actionSetState(pThis, ACT_STATE_ITX); + break; + case RS_RET_SUSPENDED: + actionRetry(pThis); + break; + case RS_RET_DISABLE_ACTION: + actionDisable(pThis); + break; + default:FINALIZE; + } } finalize_it: @@ -701,6 +717,7 @@ actionCallDoAction(action_t *pThis, msg_t *pMsg) pThis->bHadAutoCommit = 0; iRet = pThis->pMod->mod.om.doAction(pThis->ppMsgs, pMsg->msgFlags, pThis->pModData); +dbgprintf("XXX: result of doAction: %d\n", iRet); switch(iRet) { case RS_RET_OK: actionCommitted(pThis); @@ -891,6 +908,7 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem, int *pbShutdownImmedi bDone = 0; do { localRet = tryDoAction(pAction, pBatch, &nElem, pbShutdownImmediate); +dbgprintf("submitBatch: localRet %d\n", localRet); if(localRet == RS_RET_FORCE_TERM) FINALIZE; if( localRet == RS_RET_OK diff --git a/doc/action_state.dot b/doc/action_state.dot index d56d9da0..2f36d8da 100644 --- a/doc/action_state.dot +++ b/doc/action_state.dot @@ -20,6 +20,7 @@ digraph msgState { susp [label="suspended"] rdy -> itx [label="transaction begins"] + rdy -> rtry [label="begin tx\nerror"] itx -> itx [label="success"] itx -> comm [label="commit\n(caller or auto)"] itx -> rtry [label="error"] -- cgit From d3592db45bfc4939c2d72c73a42ed9f79cb64052 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 16 Dec 2009 16:21:52 +0100 Subject: bugfix: ompgsql did not properly check the server connection in tryResume() what could lead to rsyslog running in a thight loop. Also did some code cleanup of previous patch. --- ChangeLog | 4 ++++ action.c | 8 -------- plugins/ompgsql/ompgsql.c | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 37f04988..a52261de 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ --------------------------------------------------------------------------- Version 5.3.6 [BETA] (rgerhards), 2009-11-?? +- bugfix: ompgsql did not properly check the server connection in + tryResume(), which could lead to rsyslog running in a thight loop +- bugfix: suspension during beginTransaction() was not properly handled + by rsyslog core - bugfix: omfile output was only written when buffer was full, not at end of transaction - bugfix: commit transaction was not properly conveyed to message layer, diff --git a/action.c b/action.c index 5724de2f..67858742 100644 --- a/action.c +++ b/action.c @@ -491,17 +491,14 @@ static rsRetVal actionDoRetry(action_t *pThis, time_t ttNow) ASSERT(pThis != NULL); -dbgprintf("XXX: we are doing a retry, action state %d\n", pThis->eState); iRetries = 0; while(pThis->eState == ACT_STATE_RTRY) { iRet = pThis->pMod->tryResume(pThis->pModData); -dbgprintf("XXX: result of tryResume %d\n", iRet); if(iRet == RS_RET_OK) { actionSetState(pThis, ACT_STATE_RDY); } else if(iRet == RS_RET_SUSPENDED) { /* max retries reached? */ if((pThis->iResumeRetryCount != -1 && iRetries >= pThis->iResumeRetryCount)) { -dbgprintf("XXX: action suspeneded!\n"); actionSuspend(pThis, ttNow); } else { ++pThis->iNbrResRtry; @@ -533,7 +530,6 @@ static rsRetVal actionTryResume(action_t *pThis) ASSERT(pThis != NULL); -dbgprintf("XXX: actionTryResume, action state %d\n", pThis->eState); if(pThis->eState == ACT_STATE_SUSP) { /* if we are suspended, we need to check if the timeout expired. * for this handling, we must always obtain a fresh timestamp. We used @@ -580,7 +576,6 @@ static rsRetVal actionPrepare(action_t *pThis) */ if(pThis->eState == ACT_STATE_RDY) { iRet = pThis->pMod->mod.om.beginTransaction(pThis->pModData); -dbgprintf("XXX: actionPrepare, result of beginTranscation %d\n", iRet); switch(iRet) { case RS_RET_OK: actionSetState(pThis, ACT_STATE_ITX); @@ -717,7 +712,6 @@ actionCallDoAction(action_t *pThis, msg_t *pMsg) pThis->bHadAutoCommit = 0; iRet = pThis->pMod->mod.om.doAction(pThis->ppMsgs, pMsg->msgFlags, pThis->pModData); -dbgprintf("XXX: result of doAction: %d\n", iRet); switch(iRet) { case RS_RET_OK: actionCommitted(pThis); @@ -790,7 +784,6 @@ finishBatch(action_t *pThis, batch_t *pBatch) CHKiRet(actionPrepare(pThis)); if(pThis->eState == ACT_STATE_ITX) { iRet = pThis->pMod->mod.om.endTransaction(pThis->pModData); -dbgprintf("XXX: finishBatch, result of endTranscation %d\n", iRet); switch(iRet) { case RS_RET_OK: actionCommitted(pThis); @@ -908,7 +901,6 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem, int *pbShutdownImmedi bDone = 0; do { localRet = tryDoAction(pAction, pBatch, &nElem, pbShutdownImmediate); -dbgprintf("submitBatch: localRet %d\n", localRet); if(localRet == RS_RET_FORCE_TERM) FINALIZE; if( localRet == RS_RET_OK diff --git a/plugins/ompgsql/ompgsql.c b/plugins/ompgsql/ompgsql.c index 784a722d..c13f58e9 100644 --- a/plugins/ompgsql/ompgsql.c +++ b/plugins/ompgsql/ompgsql.c @@ -65,6 +65,8 @@ typedef struct _instanceData { } instanceData; +static rsRetVal writePgSQL(uchar *psz, instanceData *pData); + BEGINcreateInstance CODESTARTcreateInstance ENDcreateInstance @@ -170,9 +172,6 @@ tryExec(uchar *pszCmd, instanceData *pData) int bHadError = 0; /* try insert */ -BEGINfunc -RUNLOG_VAR("%p", pData->f_hpgsql); -RUNLOG_VAR("%s", pszCmd); pgRet = PQexec(pData->f_hpgsql, (char*)pszCmd); execState = PQresultStatus(pgRet); if(execState != PGRES_COMMAND_OK && execState != PGRES_TUPLES_OK) { @@ -181,7 +180,6 @@ RUNLOG_VAR("%s", pszCmd); } PQclear(pgRet); -ENDfunc return(bHadError); } @@ -193,7 +191,8 @@ ENDfunc * a sql format error - connection aborts were properly handled * before my patch. -- rgerhards, 2009-04-17 */ -rsRetVal writePgSQL(uchar *psz, instanceData *pData) +static rsRetVal +writePgSQL(uchar *psz, instanceData *pData) { int bHadError = 0; DEFiRet; @@ -231,6 +230,16 @@ BEGINtryResume CODESTARTtryResume if(pData->f_hpgsql == NULL) { iRet = initPgSQL(pData, 1); + if(iRet == RS_RET_OK) { + /* the code above seems not to actually connect to the database. As such, we do a + * dummy statement (a pointless select...) to verify the connection and return + * success only when that statemetn succeeds. Note that I am far from being a + * PostgreSQL expert, so any patch that does the desired result in a more + * intelligent way is highly welcome. -- rgerhards, 2009-12-16 + */ + iRet = writePgSQL((uchar*)"select 'a' as a", pData); + } + } ENDtryResume -- cgit From 009738a0ac6ba0dccf403f9e396095f44e4f9ac6 Mon Sep 17 00:00:00 2001 From: Naoya Nakazawa Date: Mon, 11 Jan 2010 12:34:46 +0100 Subject: fixed a memory leak when sending messages in zip-compressed format Signed-off-by: Rainer Gerhards --- ChangeLog | 2 ++ tools/omfwd.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/ChangeLog b/ChangeLog index 0f8037ff..ad94566d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,5 @@ +- fixed a memory leak when sending messages in zip-compressed format + Thanks to Naoya Nakazawa for analyzing this issue and providing a patch. --------------------------------------------------------------------------- Version 4.5.7 [v4-beta] (rgerhards), 2009-11-18 - added a so-called "On Demand Debug" mode, in which debug output can diff --git a/tools/omfwd.c b/tools/omfwd.c index fe65f515..02f19eac 100644 --- a/tools/omfwd.c +++ b/tools/omfwd.c @@ -483,6 +483,12 @@ CODESTARTdoAction } } finalize_it: +# ifdef USE_NETZIP + if(psz != (char*) ppString[0]) { + /* we need to free temporary buffer, alloced above - Naoya Nakazawa, 2010-01-11 */ + free(psz); + } +# endif ENDdoAction -- cgit From 3cb29f4e2b3053eaa3e2487161fc03fa730f5104 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 12 Jan 2010 12:45:42 +0100 Subject: doc bugfix: "-h" option no longer exists --- doc/rsyslog_conf_actions.html | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/doc/rsyslog_conf_actions.html b/doc/rsyslog_conf_actions.html index 2ef3f4b0..8c4b9cfc 100644 --- a/doc/rsyslog_conf_actions.html +++ b/doc/rsyslog_conf_actions.html @@ -98,18 +98,14 @@ done, same with /dev/console.

messages to a remote host running rsyslogd(8) and to receive messages from remote hosts. Using this feature you're able to control all syslog messages on one host, if all other machines will log remotely to that. -This tears down
-administration needs.
-
-Please note that this version of rsyslogd by default does NOT -forward messages it has received from the network to another host. -Specify the "-h" option to enable this.

+This tears down administration needs.

To forward messages to another host, prepend the hostname with the at sign ("@"). A single at sign means that messages will be forwarded via UDP protocol (the standard for syslog). If you prepend two at signs ("@@"), the messages will be transmitted via TCP. Please note that plain TCP based syslog is not officially standardized, but -most major syslogds support it (e.g. syslog-ng or WinSyslog). The +most major syslogds support it (e.g. syslog-ng or +WinSyslog). The forwarding action indicator (at-sign) can be followed by one or more options. If they are given, they must be immediately (without a space) following the final at sign and be enclosed in parenthesis. The -- cgit