From d5feea59255ac45328de6ec8e9478fc1b12b4145 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 8 Oct 2007 10:45:05 +0000 Subject: changed the threading to include one extra thread that runs the mainloop. This was done to work around a problem with malloc/free. Note that we are still running on two threads - the startup thread just waits for the new one to finish. For a description of the problem I try to work-around, please see: http://rgerhards.blogspot.com/2007/10/could-i-really-reproduce-bug.html Note that this is an experimental change, which will only stay if it proves to fix the segfault issue we are dealing with. --- Makefile.am | 4 +- net.c | 2 - syslogd.c | 152 ++++++++++++++++++++++++++++++++++++++---------------------- 3 files changed, 98 insertions(+), 60 deletions(-) diff --git a/Makefile.am b/Makefile.am index 642e7e17..4b33ce5b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -7,8 +7,8 @@ rfc3195d_SOURCES=rfc3195d.c rsyslog.h man_MANS = rfc3195d.8 rklogd.8 rsyslogd.8 rsyslog.conf.5 rsyslogd_SOURCES=syslogd.c pidfile.c template.c outchannel.c stringbuf.c srUtils.c parse.c syslogd-types.h template.h outchannel.h syslogd.h stringbuf.h parse.h srUtils.h liblogging-stub.h net.c net.h msg.c msg.h omshell.c omshell.h omusrmsg.c omusrmsg.h omfwd.c omfwd.h tcpsyslog.c tcpsyslog.h omfile.h omfile.c omdiscard.c omdiscard.h modules.c modules.h module-template.h objomsr.c objomsr.h cfsysline.c cfsysline.h linkedlist.c linkedlist.h iminternal.c iminternal.h action.c action.h -rsyslogd_CPPFLAGS=-D_PATH_MODDIR=\"$(pkglibdir)/\" -rsyslogd_LDADD=$(zlib_libs) $(pthreads_libs) -ldl +rsyslogd_CPPFLAGS=-O1 -fmudflapth -D_PATH_MODDIR=\"$(pkglibdir)/\" +rsyslogd_LDADD=$(zlib_libs) $(pthreads_libs) -ldl -lmudflapth rsyslogd_LDFLAGS=-export-dynamic EXTRA_DIST = doc/bugs.html doc/features.html doc/generic_design.html doc/history.html doc/how2help.html doc/install.html doc/ipv6.html doc/manual.html doc/property_replacer.html doc/rsyslog_conf.html doc/rsyslog_mysql.html doc/rsyslog_packages.html doc/rsyslog_php_syslog_ng.html doc/rsyslog_recording_pri.html doc/rsyslog_stunnel.html doc/status.html doc/syslog-protocol.html doc/version_naming.html doc/contributors.html redhat/rsyslog.conf redhat/rsyslog.init redhat/rsyslog.log redhat/rsyslog.sysconfig freebsd/rsyslogd slackware/rc.rsyslogd rfc3195d.8 rklogd.8 rsyslogd.8 rsyslog.conf.5 contrib/README doc/rsconf1_actionexeconlyifpreviousissuspended.html doc/rsconf1_actionresumeinterval.html doc/rsconf1_allowedsender.html doc/rsconf1_controlcharacterescapeprefix.html doc/rsconf1_debugprintcfsyslinehandlerlist.html doc/rsconf1_debugprintmodulelist.html doc/rsconf1_debugprinttemplatelist.html doc/rsconf1_dircreatemode.html doc/rsconf1_dirgroup.html doc/rsconf1_dirowner.html doc/rsconf1_dropmsgswithmaliciousdnsptrrecords.html doc/rsconf1_droptrailinglfonreception.html doc/rsconf1_dynafilecachesize.html doc/rsconf1_escapecontrolcharactersonreceive.html doc/rsconf1_failonchownfailure.html doc/rsconf1_filecreatemode.html doc/rsconf1_filegroup.html doc/rsconf1_fileowner.html doc/rsconf1_includeconfig.html doc/rsconf1_mainmsgqueuesize.html doc/rsconf1_modload.html doc/rsconf1_repeatedmsgreduction.html doc/rsconf1_resetconfigvariables.html doc/rsconf1_umask.html plugins/ommysql/ommysql.tar.gz diff --git a/net.c b/net.c index b6539846..bf20b9eb 100644 --- a/net.c +++ b/net.c @@ -190,7 +190,6 @@ rsRetVal gethname(struct sockaddr_storage *f, uchar *pszHostFQDN) } finalize_it: -dbgprintf("hname() iRet: %d\n", iRet); return iRet; } @@ -281,7 +280,6 @@ rsRetVal cvthname(struct sockaddr_storage *f, uchar *pszHost, uchar *pszHostFQDN } finalize_it: -dbgprintf("cvthname() iRet: %d\n", iRet); return iRet; } diff --git a/syslogd.c b/syslogd.c index 8e0d111e..2c174b39 100644 --- a/syslogd.c +++ b/syslogd.c @@ -510,6 +510,8 @@ static struct code FacNames[] = { {NULL, -1}, }; +static pid_t ppid; /* This is a quick and dirty hack used for spliting main/startup thread */ + /* global variables for config file state */ static int bDropTrailingLF = 1; /* drop trailing LF's on reception? */ int Debug; /* debug flag - read-only after startup */ @@ -6054,12 +6056,89 @@ static void printVersion(void) } +/* This is a special function to run all those things (namely the inputs) that + * were uses to run on the startup thread. I discovered a problem with malloc/free + * when used in different threads. + * See http://rgerhards.blogspot.com/2007/10/could-i-really-reproduce-bug.html + * This is now a work-around, in which I create a new thread to do all the work. If + * the malloc/free behaviour is relly what I described, then this should fix the + * segfault issue... + * RGerhards, 2007-10-08 + */ +static void *mainThread() +{ + DEFiRet; + uchar *pTmp; + sigset_t sigSet; + + /* doing some core initializations */ + if((iRet = modInitIminternal()) != RS_RET_OK) { + fprintf(stderr, "fatal error: could not initialize errbuf object (error code %d).\n", + iRet); + exit(1); /* "good" exit, leaving at init for fatal error */ + } + + if((iRet = loadBuildInModules()) != RS_RET_OK) { + fprintf(stderr, "fatal error: could not activate built-in modules. Error code %d.\n", + iRet); + exit(1); /* "good" exit, leaving at init for fatal error */ + } + + /* Block signals, all are delivered to the startup thread. + * TODO: reconsider SIGUSR1 and alarm(), which we may need to interrupt() + * the select call. For the time being, its acceptable (after all, we are right + * now doing a tester...). rgerhards, 2007-10-08 + */ + //sigfillset(&sigSet); + //pthread_sigmask(SIG_BLOCK, &sigSet, NULL); + + /* initialize the default templates + * we use template names with a SP in front - these + * can NOT be generated via the configuration file + */ + pTmp = template_TraditionalFormat; + tplAddLine(" TradFmt", &pTmp); + pTmp = template_WallFmt; + tplAddLine(" WallFmt", &pTmp); + pTmp = template_StdFwdFmt; + tplAddLine(" StdFwdFmt", &pTmp); + pTmp = template_StdUsrMsgFmt; + tplAddLine(" StdUsrMsgFmt", &pTmp); + pTmp = template_StdDBFmt; + tplLastStaticInit(tplAddLine(" StdDBFmt", &pTmp)); + + dbgprintf("Starting.\n"); + init(); + if(Debug) { + dbgprintf("Debugging enabled, SIGUSR1 to turn off debugging.\n"); + debugging_on = 1; + } + /* Send a signal to the parent so it can terminate. + */ + if (myPid != ppid) + kill (ppid, SIGTERM); + + /* END OF INTIALIZATION + * ... but keep in mind that we might do a restart and thus init() might + * be called again. If that happens, we must shut down the worker thread, + * do the init() and then restart things. + * rgerhards, 2005-10-24 + */ + + mainloop(); + + /* do any de-init's that need to be done AFTER this comment */ + die(bFinished); + + pthread_exit(0); +} + + /* This is the main entry point into rsyslogd. Over time, we should try to * modularize it a bit more... */ int main(int argc, char **argv) { - DEFiRet; register int i; register char *p; int num_fds; @@ -6067,15 +6146,16 @@ int main(int argc, char **argv) struct hostent *hent; extern int optind; extern char *optarg; - uchar *pTmp; struct sigaction sigAct; + pthread_t thrdMain; + sigset_t sigSet; #ifdef MTRACE mtrace(); /* this is a debug aid for leak detection - either remove * or put in conditional compilation. 2005-01-18 RGerhards */ #endif - pid_t ppid = getpid(); + ppid = getpid(); if(chdir ("/") != 0) fprintf(stderr, "Can not do 'cd /' - still trying to run\n"); @@ -6084,19 +6164,6 @@ int main(int argc, char **argv) funix[i] = -1; } - /* doing some core initializations */ - if((iRet = modInitIminternal()) != RS_RET_OK) { - fprintf(stderr, "fatal error: could not initialize errbuf object (error code %d).\n", - iRet); - exit(1); /* "good" exit, leaving at init for fatal error */ - } - - if((iRet = loadBuildInModules()) != RS_RET_OK) { - fprintf(stderr, "fatal error: could not activate built-in modules. Error code %d.\n", - iRet); - exit(1); /* "good" exit, leaving at init for fatal error */ - } - /* END core initializations */ while ((ch = getopt(argc, argv, "46Aa:dehi:f:l:m:nop:r::s:t:u:vwx")) != EOF) { @@ -6267,20 +6334,6 @@ int main(int argc, char **argv) } /* if ( !Debug ) */ myPid = getpid(); /* save our pid for further testing (also used for messages) */ - /* initialize the default templates - * we use template names with a SP in front - these - * can NOT be generated via the configuration file - */ - pTmp = template_TraditionalFormat; - tplAddLine(" TradFmt", &pTmp); - pTmp = template_WallFmt; - tplAddLine(" WallFmt", &pTmp); - pTmp = template_StdFwdFmt; - tplAddLine(" StdFwdFmt", &pTmp); - pTmp = template_StdUsrMsgFmt; - tplAddLine(" StdUsrMsgFmt", &pTmp); - pTmp = template_StdDBFmt; - tplLastStaticInit(tplAddLine(" StdDBFmt", &pTmp)); gethostname(LocalHostName, sizeof(LocalHostName)); if ( (p = strchr(LocalHostName, '.')) ) { @@ -6341,37 +6394,24 @@ int main(int argc, char **argv) sigaction(SIGXFSZ, &sigAct, NULL); /* do not abort if 2gig file limit is hit */ (void) alarm(TIMERINTVL); - dbgprintf("Starting.\n"); - init(); - if(Debug) { - dbgprintf("Debugging enabled, SIGUSR1 to turn off debugging.\n"); - debugging_on = 1; - } - /* Send a signal to the parent so it can terminate. + i = pthread_create(&thrdMain, NULL, mainThread, NULL); + dbgprintf("\"main\" thread started with state %d.\n", i); + + /* we block all signals - they will be processed by the "main"-thread. This most + * closely resembles previous behaviour. TODO: think about optimizing it, some + * signals may better be delivered here. rgerhards, 2007-10-08 */ - if (myPid != ppid) - kill (ppid, SIGTERM); - /* END OF INTIALIZATION - * ... but keep in mind that we might do a restart and thus init() might - * be called again. If that happens, we must shut down all active threads, - * do the init() and then restart things. - * rgerhards, 2005-10-24 + sigfillset(&sigSet); + pthread_sigmask(SIG_BLOCK, &sigSet, NULL); + + /* see comment in mainThread on why we start thread and then immediately + * do a blocking wait on it - it makese sense... ;) rgerhards, 2007-10-08 */ + pthread_join(thrdMain, NULL); - mainloop(); - - /* do any de-init's that need to be done AFTER this comment */ - - die(bFinished); return 0; } -/* - * Local variables: - * c-indent-level: 8 - * c-basic-offset: 8 - * tab-width: 8 - * End: - * vi:set ai: +/* vi:set ai: */ -- cgit