From 28c44e9a7bf4f99279af94a96bac42d0379b27d0 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 10 Jan 2008 14:27:26 +0000 Subject: - fixed a bug that caused a segfault on startup when no $WorkDir directive was specified in rsyslog.conf - fixed a bug that caused a segfault on queues with types other than "disk" - removed the now longer needed thread TermSyncTool --- ChangeLog | 4 ++++ module-template.h | 12 ------------ modules.c | 3 --- modules.h | 1 - plugins/imklog/imklog.c | 1 - plugins/immark/immark.c | 1 - plugins/imtcp/imtcp.c | 1 - plugins/imudp/imudp.c | 1 - plugins/imuxsock/imuxsock.c | 1 - queue.c | 17 +++++++++++++---- syslogd.c | 15 +++++++++++++-- threads.c | 25 +++++-------------------- threads.h | 9 +-------- 13 files changed, 36 insertions(+), 55 deletions(-) diff --git a/ChangeLog b/ChangeLog index ee1bb2e9..92987c3c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ --------------------------------------------------------------------------- +Version 3.10.2 (rgerhards), 2008-01-1? +- fixed a bug that caused a segfault on startup when no $WorkDir directive + was specified in rsyslog.conf +--------------------------------------------------------------------------- Version 3.10.1 (rgerhards), 2008-01-10 - implemented the "disk" queue mode. However, it currently is of very limited use, because it does not support persistence over rsyslogd diff --git a/module-template.h b/module-template.h index 3b6ebb69..ecd6fe6d 100644 --- a/module-template.h +++ b/module-template.h @@ -334,8 +334,6 @@ static rsRetVal queryEtryPt(uchar *name, rsRetVal (**pEtryPoint)())\ CODEqueryEtryPt_STD_MOD_QUERIES \ else if(!strcmp((char*) name, "runInput")) {\ *pEtryPoint = runInput;\ - } else if(!strcmp((char*) name, "getTermSyncType")) {\ - *pEtryPoint = modGetTermSyncType;\ } else if(!strcmp((char*) name, "willRun")) {\ *pEtryPoint = willRun;\ } else if(!strcmp((char*) name, "afterRun")) {\ @@ -471,16 +469,6 @@ static rsRetVal afterRun(void)\ } -/* method to return which termination sync method is used by this module. - */ -#define TERM_SYNC_TYPE(x) \ -static rsRetVal modGetTermSyncType(eTermSyncType_t *pTermSync)\ -{\ - assert(pTermSync != NULL); \ - *pTermSync = (x);\ - return RS_RET_OK;\ -} - /* * vi:set ai: */ diff --git a/modules.c b/modules.c index 6e7318d3..b8fc54ef 100644 --- a/modules.c +++ b/modules.c @@ -216,7 +216,6 @@ rsRetVal doModInit(rsRetVal (*modInit)(int, int*, rsRetVal(**)(), rsRetVal(*)()) DEFiRet; modInfo_t *pNew = NULL; rsRetVal (*modGetType)(eModType_t *pType); - rsRetVal (*modGetTermSyncType)(eTermSyncType_t *pType); assert(modInit != NULL); @@ -256,8 +255,6 @@ rsRetVal doModInit(rsRetVal (*modInit)(int, int*, rsRetVal(**)(), rsRetVal(*)()) /* ... and now the module-specific interfaces */ switch(pNew->eType) { case eMOD_IN: - CHKiRet((*pNew->modQueryEtryPt)((uchar*)"getTermSyncType", &modGetTermSyncType)); - CHKiRet((iRet = (*modGetTermSyncType)(&pNew->mod.im.eTermSyncType)) != RS_RET_OK); CHKiRet((*pNew->modQueryEtryPt)((uchar*)"runInput", &pNew->mod.im.runInput)); CHKiRet((*pNew->modQueryEtryPt)((uchar*)"willRun", &pNew->mod.im.willRun)); CHKiRet((*pNew->modQueryEtryPt)((uchar*)"afterRun", &pNew->mod.im.afterRun)); diff --git a/modules.h b/modules.h index 0fbc7a6b..c3de45d2 100644 --- a/modules.h +++ b/modules.h @@ -75,7 +75,6 @@ typedef struct moduleInfo { /* TODO: pass pointer to msg submit function to IM rger, 2007-12-14 */ union { struct {/* data for input modules */ - eTermSyncType_t eTermSyncType; rsRetVal (*runInput)(thrdInfo_t*); /* function to gather input and submit to queue */ rsRetVal (*willRun)(void); /* function to gather input and submit to queue */ rsRetVal (*afterRun)(thrdInfo_t*); /* function to gather input and submit to queue */ diff --git a/plugins/imklog/imklog.c b/plugins/imklog/imklog.c index 8ed900dd..427d1b5e 100644 --- a/plugins/imklog/imklog.c +++ b/plugins/imklog/imklog.c @@ -43,7 +43,6 @@ #include "imklog.h" MODULE_TYPE_INPUT -TERM_SYNC_TYPE(eTermSync_NONE) /* Module static data */ DEF_IMOD_STATIC_DATA diff --git a/plugins/immark/immark.c b/plugins/immark/immark.c index 973821ad..f21bfd7f 100644 --- a/plugins/immark/immark.c +++ b/plugins/immark/immark.c @@ -42,7 +42,6 @@ #include "module-template.h" MODULE_TYPE_INPUT -TERM_SYNC_TYPE(eTermSync_SIGNAL) /* defines */ #define DEFAULT_MARK_PERIOD (20 * 60) diff --git a/plugins/imtcp/imtcp.c b/plugins/imtcp/imtcp.c index 579885ca..01852044 100644 --- a/plugins/imtcp/imtcp.c +++ b/plugins/imtcp/imtcp.c @@ -38,7 +38,6 @@ #include "tcpsyslog.h" MODULE_TYPE_INPUT -TERM_SYNC_TYPE(eTermSync_NONE) /* defines */ diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 0251a51a..baa193d5 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -39,7 +39,6 @@ #include "module-template.h" MODULE_TYPE_INPUT -TERM_SYNC_TYPE(eTermSync_NONE) /* defines */ diff --git a/plugins/imuxsock/imuxsock.c b/plugins/imuxsock/imuxsock.c index b51c24d1..863f0d50 100644 --- a/plugins/imuxsock/imuxsock.c +++ b/plugins/imuxsock/imuxsock.c @@ -40,7 +40,6 @@ #include "module-template.h" MODULE_TYPE_INPUT -TERM_SYNC_TYPE(eTermSync_NONE) /* defines */ #define MAXFUNIX 20 diff --git a/queue.c b/queue.c index bfd456f4..f64ec2c8 100644 --- a/queue.c +++ b/queue.c @@ -420,13 +420,16 @@ rsRetVal queueConstruct(queue_t **ppThis, queueType_t qType, int iWorkerThreads, assert(pConsumer != NULL); assert(iWorkerThreads >= 0); +dbgprintf("queueConstruct 0\n"); if((pThis = (queue_t *)calloc(1, sizeof(queue_t))) == NULL) { ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); } +dbgprintf("queueConstruct 0a\n"); /* we have an object, so let's fill the properties */ if((pThis->pszSpoolDir = (uchar*) strdup((char*)glblGetWorkDir())) == NULL) ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); +dbgprintf("queueConstruct 1\n"); pThis->lenSpoolDir = strlen((char*)pThis->pszSpoolDir); pThis->iMaxFileSize = 1024 * 1024; /* default is 1 MiB */ @@ -444,6 +447,7 @@ rsRetVal queueConstruct(queue_t **ppThis, queueType_t qType, int iWorkerThreads, pThis->pszFilePrefix = NULL; pThis->qType = qType; +dbgprintf("queueConstruct 2\n"); /* set type-specific handlers and other very type-specific things (we can not totally hide it...) */ switch(qType) { case QUEUETYPE_FIXED_ARRAY: @@ -474,11 +478,14 @@ rsRetVal queueConstruct(queue_t **ppThis, queueType_t qType, int iWorkerThreads, break; } +dbgprintf("queueConstruct 3\n"); /* call type-specific constructor */ CHKiRet(pThis->qConstruct(pThis)); finalize_it: +dbgprintf("queueConstruct 4\n"); OBJCONSTRUCT_CHECK_SUCCESS_AND_CLEANUP +dbgprintf("queueConstruct 5\n"); return iRet; } @@ -564,8 +571,10 @@ rsRetVal queueSetFilePrefix(queue_t *pThis, uchar *pszPrefix, size_t iLenPrefix) { DEFiRet; - CHKiRet(strmSetFilePrefix(pThis->tVars.disk.pWrite, pszPrefix, iLenPrefix)); - CHKiRet(strmSetFilePrefix(pThis->tVars.disk.pRead, pszPrefix, iLenPrefix)); + if(pThis->qType == QUEUETYPE_DISK) { + CHKiRet(strmSetFilePrefix(pThis->tVars.disk.pWrite, pszPrefix, iLenPrefix)); + CHKiRet(strmSetFilePrefix(pThis->tVars.disk.pRead, pszPrefix, iLenPrefix)); + } finalize_it: return iRet; } @@ -585,8 +594,8 @@ queueSetMaxFileSize(queue_t *pThis, size_t iMaxFileSize) } pThis->iMaxFileSize = iMaxFileSize; -// TODO: check queue mode! also in other places!!! - CHKiRet(strmSetiMaxFileSize(pThis->tVars.disk.pWrite, iMaxFileSize)); + if(pThis->qType == QUEUETYPE_DISK) + CHKiRet(strmSetiMaxFileSize(pThis->tVars.disk.pWrite, iMaxFileSize)); finalize_it: return iRet; diff --git a/syslogd.c b/syslogd.c index d949b72b..e1fcb056 100644 --- a/syslogd.c +++ b/syslogd.c @@ -3244,7 +3244,7 @@ startInputModules(void) while(pMod != NULL) { if((iRet = pMod->mod.im.willRun()) == RS_RET_OK) { /* activate here */ - thrdCreate(pMod->mod.im.runInput, pMod->mod.im.eTermSyncType, pMod->mod.im.afterRun); + thrdCreate(pMod->mod.im.runInput, pMod->mod.im.afterRun); } else { dbgprintf("module %lx will not run, iRet %d\n", (unsigned long) pMod, iRet); } @@ -3349,6 +3349,12 @@ init(void) iMainMsgQueueNumWorkers = 1; } + if(MainMsgQueType == QUEUETYPE_DISK && pszWorkDir == NULL) { + fprintf(stderr, "No $WorkDirectory specified - can not run main message queue in 'disk' mode. " + "Using 'FixedArray' instead.\n"); + MainMsgQueType = QUEUETYPE_FIXED_ARRAY; + } + /* switch the message object to threaded operation, if necessary */ if(MainMsgQueType == QUEUETYPE_DIRECT || iMainMsgQueueNumWorkers > 1) { MsgEnableThreadSafety(); @@ -3360,6 +3366,7 @@ init(void) fprintf(stderr, "fatal error %d: could not create message queue - rsyslogd can not run!\n", iRet); exit(1); } +dbgprintf("queue 1 \n"); /* ... set some properties ... */ # define setQPROP(func, directive, data) \ CHKiRet_Hdlr(func(pMsgQueue, data)) { \ @@ -3371,18 +3378,21 @@ init(void) } setQPROP(queueSetMaxFileSize, "$MainMsgQueueFileSize", iMainMsgQueMaxFileSize); - setQPROPstr(queueSetFilePrefix, "$MainMsgQueueFilePrefix", +dbgprintf("queue 2 \n"); + setQPROPstr(queueSetFilePrefix, "$MainMsgQueueFileName", (pszMainMsgQFName == NULL ? (uchar*) "mainq" : pszMainMsgQFName)); # undef setQPROP # undef setQPROPstr +dbgprintf("try start queue \n"); /* ... and finally start the queue! */ CHKiRet_Hdlr(queueStart(pMsgQueue)) { /* no queue is fatal, we need to give up in that case... */ fprintf(stderr, "fatal error %d: could not start message queue - rsyslogd can not run!\n", iRet); exit(1); } +dbgprintf("queue running\n"); Initialized = 1; bHaveMainQueue = (MainMsgQueType == QUEUETYPE_DIRECT) ? 0 : 1; @@ -3395,6 +3405,7 @@ init(void) */ startInputModules(); +dbgprintf("inputs running\n"); if(Debug) { dbgPrintInitInfo(); } diff --git a/threads.c b/threads.c index 24cae97f..2166f4bb 100644 --- a/threads.c +++ b/threads.c @@ -86,29 +86,15 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis) } -/* terminate a thread gracefully. It's termination sync state is taken into - * account. +/* terminate a thread gracefully. */ rsRetVal thrdTerminate(thrdInfo_t *pThis) { assert(pThis != NULL); -dbgprintf("Terminate thread %lx via method %d\n", pThis->thrdID, pThis->eTermTool); - if(pThis->eTermTool == eTermSync_SIGNAL) { - /* we first wait for the thread to reach a point in execution where it - * is safe to terminate it. - * TODO: TIMEOUT! - */ - pthread_mutex_lock(pThis->mutTermOK); - pThis->bShallStop = 1; /* request termination */ - pthread_kill(pThis->thrdID, SIGUSR2); /* get thread out ouf blocking calls */ - pthread_join(pThis->thrdID, NULL); - pthread_mutex_unlock(pThis->mutTermOK); /* cleanup... */ - /* TODO: TIMEOUT! */ - } else if(pThis->eTermTool == eTermSync_NONE) { - pthread_cancel(pThis->thrdID); - pthread_join(pThis->thrdID, NULL); /* wait for cancel to complete */ - } +dbgprintf("Terminate thread %lx\n", pThis->thrdID); + pthread_cancel(pThis->thrdID); + pthread_join(pThis->thrdID, NULL); /* wait for cancel to complete */ pThis->bIsActive = 0; /* call cleanup function, if any */ @@ -164,7 +150,7 @@ static void* thrdStarter(void *arg) * executing threads. It is added at the end of the list. * rgerhards, 2007-12-14 */ -rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), eTermSyncType_t eTermSyncType, rsRetVal(*afterRun)(thrdInfo_t *)) +rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdInfo_t *)) { DEFiRet; thrdInfo_t *pThis; @@ -173,7 +159,6 @@ rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), eTermSyncType_t eTermSync assert(thrdMain != NULL); CHKiRet(thrdConstruct(&pThis)); - pThis->eTermTool = eTermSync_NONE; // eTermSyncType; TODO: review pThis->bIsActive = 1; pThis->pUsrThrdMain = thrdMain; pThis->pAfterRun = afterRun; diff --git a/threads.h b/threads.h index b0cc9221..dc937def 100644 --- a/threads.h +++ b/threads.h @@ -24,15 +24,8 @@ #define THREADS_H_INCLUDED -/* type of sync tools for terminating the thread */ -typedef enum eTermSyncType { - eTermSync_NONE = 0, /* no cleanup necessary, just cancel thread */ - eTermSync_SIGNAL /* termination via pthread_kill() */ -} eTermSyncType_t; - /* the thread object */ typedef struct thrdInfo { - eTermSyncType_t eTermTool; pthread_mutex_t *mutTermOK; /* Is it ok to terminate that thread now? */ int bIsActive; /* Is thread running? */ int bShallStop; /* set to 1 if the thread should be stopped ? */ @@ -46,7 +39,7 @@ rsRetVal thrdExit(void); rsRetVal thrdInit(void); rsRetVal thrdTerminate(thrdInfo_t *pThis); rsRetVal thrdTerminateAll(void); -rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), eTermSyncType_t eTermSyncType, rsRetVal(*afterRun)(thrdInfo_t *)); +rsRetVal thrdCreate(rsRetVal (*thrdMain)(thrdInfo_t*), rsRetVal(*afterRun)(thrdInfo_t *)); rsRetVal thrdSleep(thrdInfo_t *pThis, int iSeconds, int iuSeconds); /* macros (replace inline functions) */ -- cgit