From c53ca3a23429e750aeb6ab1c3600ae8ecb3c8ac6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 23 Jun 2009 18:23:02 +0200 Subject: fixed a race condition: generated msg string store must be mutex protected the string area that is used to build the string being passed to the output module is now part of the action structure. As such, access to it must also be guarded by the action mutex (an even more optimal solution may be to store it in thread-local storage, but there always must remain some room for improvement ;)). --- action.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'action.c') diff --git a/action.c b/action.c index 2040d6bd..01bbfd13 100644 --- a/action.c +++ b/action.c @@ -460,6 +460,16 @@ actionCallDoAction(action_t *pAction, msg_t *pMsg) ASSERT(pAction != NULL); + /* We now must guard the output module against execution by multiple threads. The + * plugin interface specifies that output modules must not be thread-safe (except + * if they notify us they are - functionality not yet implemented...). + * rgerhards, 2008-01-30 + */ + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); + d_pthread_mutex_lock(&pAction->mutActExec); + pthread_cleanup_push(mutexCancelCleanup, &pAction->mutActExec); + pthread_setcancelstate(iCancelStateSave, NULL); + /* here we must loop to process all requested strings */ for(i = 0 ; i < pAction->iNumTpls ; ++i) { switch(pAction->eParamPassing) { @@ -472,16 +482,8 @@ actionCallDoAction(action_t *pAction, msg_t *pMsg) default:assert(0); /* software bug if this happens! */ } } + iRetries = 0; - /* We now must guard the output module against execution by multiple threads. The - * plugin interface specifies that output modules must not be thread-safe (except - * if they notify us they are - functionality not yet implemented...). - * rgerhards, 2008-01-30 - */ - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &iCancelStateSave); - d_pthread_mutex_lock(&pAction->mutActExec); - pthread_cleanup_push(mutexCancelCleanup, &pAction->mutActExec); - pthread_setcancelstate(iCancelStateSave, NULL); do { /* on first invocation, this if should never be true. We just put it at the top * of the loop so that processing (and code) is simplified. This code is actually @@ -520,8 +522,6 @@ actionCallDoAction(action_t *pAction, msg_t *pMsg) pAction->bEnabled = 0; /* that's it... */ } - pthread_cleanup_pop(1); /* unlock mutex */ - finalize_it: /* cleanup */ for(i = 0 ; i < pAction->iNumTpls ; ++i) { @@ -544,6 +544,8 @@ finalize_it: } } + pthread_cleanup_pop(1); /* unlock mutex */ + msgDestruct(&pMsg); /* we are now finished with the message */ RETiRet; } -- cgit