From a5efd74c7161682a4ea8829f598db46bd836ab69 Mon Sep 17 00:00:00 2001 From: dockes Date: Mon, 21 May 2007 13:30:22 +0000 Subject: [PATCH] make sure signals are only handled by the main thread. Fix bus error on rclmon exit (double delete) --- src/common/rclinit.cpp | 25 ++++++++++++++++++++++++- src/common/rclinit.h | 8 +++++++- src/index/rclmonprc.cpp | 13 ++++++++++--- src/index/rclmonrcv.cpp | 6 ++++-- src/index/recollindex.cpp | 19 ++++++++----------- src/qtgui/idxthread.cpp | 8 +++++--- src/qtgui/main.cpp | 14 ++++++-------- src/rcldb/rcldb.cpp | 4 +++- src/utils/execmd.cpp | 13 ++----------- src/utils/execmd.h | 6 +++++- 10 files changed, 74 insertions(+), 42 deletions(-) diff --git a/src/common/rclinit.cpp b/src/common/rclinit.cpp index bd44ba66..e753e1b0 100644 --- a/src/common/rclinit.cpp +++ b/src/common/rclinit.cpp @@ -1,5 +1,5 @@ #ifndef lint -static char rcsid[] = "@(#$Id: rclinit.cpp,v 1.8 2006-11-08 15:34:20 dockes Exp $ (C) 2004 J.F.Dockes"; +static char rcsid[] = "@(#$Id: rclinit.cpp,v 1.9 2007-05-21 13:30:21 dockes Exp $ (C) 2004 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -21,6 +21,7 @@ static char rcsid[] = "@(#$Id: rclinit.cpp,v 1.8 2006-11-08 15:34:20 dockes Exp #include #include #include +#include #include "debuglog.h" #include "rclconfig.h" @@ -36,6 +37,16 @@ RclConfig *recollinit(RclInitFlags flags, if (cleanup) atexit(cleanup); + // We ignore SIGPIPE always. All pieces of code which can write to a pipe + // must check write() return values. + signal(SIGPIPE, SIG_IGN); + + // We block SIGCLD globally. We intend to properly wait for our children + sigset_t sset; + sigemptyset(&sset); + sigaddset(&sset, SIGCHLD); + pthread_sigmask(SIG_BLOCK, &sset, 0); + if (sigcleanup) { for (unsigned int i = 0; i < sizeof(catchedSigs) / sizeof(int); i++) if (signal(catchedSigs[i], SIG_IGN) != SIG_IGN) @@ -86,3 +97,15 @@ RclConfig *recollinit(RclInitFlags flags, return config; } + +// Signals are handled by the main thread. All others should call this routine +// to block possible signals +void recoll_threadinit() +{ + sigset_t sset; + sigemptyset(&sset); + + for (unsigned int i = 0; i < sizeof(catchedSigs) / sizeof(int); i++) + sigaddset(&sset, catchedSigs[i]); + pthread_sigmask(SIG_BLOCK, &sset, 0); +} diff --git a/src/common/rclinit.h b/src/common/rclinit.h index 6991d88a..1e8866ba 100644 --- a/src/common/rclinit.h +++ b/src/common/rclinit.h @@ -16,7 +16,7 @@ */ #ifndef _RCLINIT_H_INCLUDED_ #define _RCLINIT_H_INCLUDED_ -/* @(#$Id: rclinit.h,v 1.5 2006-11-08 07:22:14 dockes Exp $ (C) 2004 J.F.Dockes */ +/* @(#$Id: rclinit.h,v 1.6 2007-05-21 13:30:21 dockes Exp $ (C) 2004 J.F.Dockes */ #include #ifndef NO_NAMESPACES @@ -26,6 +26,10 @@ using std::string; class RclConfig; /** * Initialize by reading configuration, opening log file, etc. + * + * This must be called from the main thread before starting any others. It sets + * up the global signal handling. other threads must call recoll_threadinit() + * when starting. * * @param flags misc modifiers * @param cleanup function to call before exiting (atexit) @@ -46,5 +50,7 @@ inline RclConfig *recollinit(void (*cleanup)(void), void (*sigcleanup)(int), string &reason, const string *argcnf = 0) { return recollinit(RCLINIT_NONE, cleanup, sigcleanup, reason, argcnf); } +// Threads need to call this. The main thread handles all signals. +extern void recoll_threadinit(); #endif /* _RCLINIT_H_INCLUDED_ */ diff --git a/src/index/rclmonprc.cpp b/src/index/rclmonprc.cpp index a31e1f54..618d9b47 100644 --- a/src/index/rclmonprc.cpp +++ b/src/index/rclmonprc.cpp @@ -2,7 +2,7 @@ #ifdef RCL_MONITOR #ifndef lint -static char rcsid[] = "@(#$Id: rclmonprc.cpp,v 1.11 2007-05-21 09:00:29 dockes Exp $ (C) 2006 J.F.Dockes"; +static char rcsid[] = "@(#$Id: rclmonprc.cpp,v 1.12 2007-05-21 13:30:21 dockes Exp $ (C) 2006 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -291,6 +291,7 @@ bool startMonitor(RclConfig *conf, int opts) } if (!rclEQ.lock()) { + LOGERR(("startMonitor: cant lock queue ???\n")); return false; } LOGDEB(("start_monitoring: entering main loop\n")); @@ -302,8 +303,11 @@ bool startMonitor(RclConfig *conf, int opts) while (rclEQ.wait(2, &timedout)) { // Queue is locked. - if (!rclEQ.ok()) + if (!rclEQ.ok()) { + rclEQ.unlock(); break; + } + list modified; list deleted; @@ -339,7 +343,7 @@ bool startMonitor(RclConfig *conf, int opts) didsomething = true; } - // Recreate the auxiliary dbs every hour. + // Recreate the auxiliary dbs every hour at most. const int auxinterval = 60 *60; if (didsomething && time(0) - lastauxtime > auxinterval) { lastauxtime = time(0); @@ -355,6 +359,9 @@ bool startMonitor(RclConfig *conf, int opts) // Lock queue before waiting again rclEQ.lock(); } + rclEQ.setTerminate(); + // Wait for receiver thread before returning + pthread_join(rcv_thrid, 0); LOGDEB(("Monitor: returning\n")); return true; } diff --git a/src/index/rclmonrcv.cpp b/src/index/rclmonrcv.cpp index 44e896bf..be259608 100644 --- a/src/index/rclmonrcv.cpp +++ b/src/index/rclmonrcv.cpp @@ -1,7 +1,7 @@ #include "autoconfig.h" #ifdef RCL_MONITOR #ifndef lint -static char rcsid[] = "@(#$Id: rclmonrcv.cpp,v 1.10 2007-02-02 10:12:58 dockes Exp $ (C) 2006 J.F.Dockes"; +static char rcsid[] = "@(#$Id: rclmonrcv.cpp,v 1.11 2007-05-21 13:30:21 dockes Exp $ (C) 2006 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -25,6 +25,7 @@ static char rcsid[] = "@(#$Id: rclmonrcv.cpp,v 1.10 2007-02-02 10:12:58 dockes E #include "debuglog.h" #include "rclmon.h" +#include "rclinit.h" #include "fstreewalk.h" #include "indexer.h" #include "pathut.h" @@ -108,6 +109,7 @@ void *rclMonRcvRun(void *q) RclMonEventQueue *queue = (RclMonEventQueue *)q; LOGDEB(("rclMonRcvRun: running\n")); + recoll_threadinit(); // Create the fam/whatever interface object RclMonitor *mon; @@ -163,8 +165,8 @@ void *rclMonRcvRun(void *q) } } - LOGINFO(("rclMonRcvRun: exiting\n")); queue->setTerminate(); + LOGINFO(("rclMonRcvRun: monrcv thread routine returning\n")); return 0; } diff --git a/src/index/recollindex.cpp b/src/index/recollindex.cpp index a2b0f813..8cf959fa 100644 --- a/src/index/recollindex.cpp +++ b/src/index/recollindex.cpp @@ -1,5 +1,5 @@ #ifndef lint -static char rcsid[] = "@(#$Id: recollindex.cpp,v 1.31 2007-02-02 10:09:10 dockes Exp $ (C) 2004 J.F.Dockes"; +static char rcsid[] = "@(#$Id: recollindex.cpp,v 1.32 2007-05-21 13:30:21 dockes Exp $ (C) 2004 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -74,8 +74,7 @@ static bool makeDbIndexer(RclConfig *config) } // Check if there is already an indexer for the right db if (dbindexer && dbindexer->getDbDir().compare(dbdir)) { - delete dbindexer; - dbindexer = 0; + deleteZ(dbindexer); } if (!dbindexer) @@ -199,10 +198,8 @@ static bool createstemdb(RclConfig *config, const string &lang) static void cleanup() { - delete confindexer; - confindexer = 0; - delete dbindexer; - dbindexer = 0; + deleteZ(confindexer); + deleteZ(dbindexer); } static const char *thisprog; @@ -360,15 +357,15 @@ int main(int argc, const char **argv) confindexer = new ConfIndexer(config, &updater); confindexer->index(rezero); - delete confindexer; + deleteZ(confindexer); int opts = RCLMON_NONE; if (op_flags & OPT_D) opts |= RCLMON_NOFORK; if (op_flags & OPT_x) opts |= RCLMON_NOX11; - if (startMonitor(config, opts)) - exit(0); - exit(1); + bool monret = startMonitor(config, opts); + MONDEB(("Monitor returned %d, exiting\n", monret)); + exit(monret == false); #endif // MONITOR #ifdef RCL_USE_ASPELL diff --git a/src/qtgui/idxthread.cpp b/src/qtgui/idxthread.cpp index 4e61ee87..6b15f482 100644 --- a/src/qtgui/idxthread.cpp +++ b/src/qtgui/idxthread.cpp @@ -24,6 +24,8 @@ #include "indexer.h" #include "debuglog.h" #include "idxthread.h" +#include "smallut.h" +#include "rclinit.h" static QMutex curfile_mutex; @@ -56,9 +58,10 @@ static int stopidxthread; void IdxThread::run() { DebugLog::getdbl()->setloglevel(loglevel); + recoll_threadinit(); for (;;) { if (stopidxthread) { - delete indexer; + deleteZ(indexer); return; } if (startindexing) { @@ -85,8 +88,7 @@ void start_idxthread(const RclConfig& cnf) // We have to make a copy of the config (setKeydir changes it during // indexation) RclConfig *myconf = new RclConfig(cnf); - ConfIndexer *ix = new ConfIndexer(myconf, &idxthread); - idxthread.indexer = ix; + idxthread.indexer = new ConfIndexer(myconf, &idxthread); idxthread.loglevel = DebugLog::getdbl()->getlevel(); idxthread.start(); } diff --git a/src/qtgui/main.cpp b/src/qtgui/main.cpp index bf9be779..8062eeba 100644 --- a/src/qtgui/main.cpp +++ b/src/qtgui/main.cpp @@ -1,5 +1,5 @@ #ifndef lint -static char rcsid[] = "@(#$Id: main.cpp,v 1.58 2007-01-08 15:21:32 dockes Exp $ (C) 2005 J.F.Dockes"; +static char rcsid[] = "@(#$Id: main.cpp,v 1.59 2007-05-21 13:30:21 dockes Exp $ (C) 2005 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -57,6 +57,7 @@ static char rcsid[] = "@(#$Id: main.cpp,v 1.58 2007-01-08 15:21:32 dockes Exp $ #ifdef RCL_USE_ASPELL #include "rclaspell.h" #endif +#include "smallut.h" #ifdef WITH_KDE static const char description[] = @@ -117,20 +118,17 @@ static void recollCleanup() LOGDEB2(("recollCleanup: stopping idx thread\n")); stop_idxthread(); LOGDEB2(("recollCleanup: closing database\n")); - delete rcldb; - rcldb = 0; - delete rclconfig; - rclconfig = 0; + deleteZ(rcldb); + deleteZ(rclconfig); #ifdef RCL_USE_ASPELL - delete aspell; - aspell = 0; + deleteZ(aspell); #endif LOGDEB2(("recollCleanup: done\n")); } static void sigcleanup(int) { - // fprintf(stderr, "sigcleanup\n"); + fprintf(stderr, "sigcleanup called\n"); // Cant call exit from here, because the atexit cleanup does some // thread stuff that we can't do from signal context. // Just set a flag and let the watchdog timer do the work diff --git a/src/rcldb/rcldb.cpp b/src/rcldb/rcldb.cpp index 33e8c420..ed40e539 100644 --- a/src/rcldb/rcldb.cpp +++ b/src/rcldb/rcldb.cpp @@ -1,5 +1,5 @@ #ifndef lint -static char rcsid[] = "@(#$Id: rcldb.cpp,v 1.107 2007-05-18 07:41:03 dockes Exp $ (C) 2004 J.F.Dockes"; +static char rcsid[] = "@(#$Id: rcldb.cpp,v 1.108 2007-05-21 13:30:21 dockes Exp $ (C) 2004 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -114,6 +114,8 @@ class Native { if (m_iswritable) LOGDEB(("Rcl::Db: xapian will close. Flush may take some time\n")); delete enquire; + if (m_iswritable) + LOGDEB(("Rcl::Db: xapian close done.\n")); } string makeAbstract(Xapian::docid id, const list& terms); diff --git a/src/utils/execmd.cpp b/src/utils/execmd.cpp index 0233e906..5a6741f8 100644 --- a/src/utils/execmd.cpp +++ b/src/utils/execmd.cpp @@ -1,5 +1,5 @@ #ifndef lint -static char rcsid[] = "@(#$Id: execmd.cpp,v 1.22 2007-02-19 18:14:13 dockes Exp $ (C) 2004 J.F.Dockes"; +static char rcsid[] = "@(#$Id: execmd.cpp,v 1.23 2007-05-21 13:30:22 dockes Exp $ (C) 2004 J.F.Dockes"; #endif /* * This program is free software; you can redistribute it and/or modify @@ -26,6 +26,7 @@ static char rcsid[] = "@(#$Id: execmd.cpp,v 1.22 2007-02-19 18:14:13 dockes Exp #include #include #include + #ifdef PUTENV_ARG_NOT_CONST #include #endif @@ -178,14 +179,6 @@ int ExecCmd::doexec(const string &cmd, const list& args, } if (e.pid) { - // Ignore SIGPIPE and block SIGCHLD in here. - void (*osig)(int); - osig = signal(SIGPIPE, SIG_IGN); - sigset_t blkcld; - sigemptyset(&blkcld); - sigaddset(&blkcld, SIGCHLD); - sigprocmask(SIG_BLOCK, &blkcld, 0); - // Father process if (input) { close(e.pipein[0]); @@ -286,8 +279,6 @@ int ExecCmd::doexec(const string &cmd, const list& args, (void)waitpid(e.pid, &status, 0); e.pid = -1; } - signal(SIGPIPE, osig); - sigprocmask(SIG_UNBLOCK, &blkcld, 0); LOGDEB1(("ExecCmd::doexec: father got status 0x%x\n", status)); return haderror ? -1 : status; diff --git a/src/utils/execmd.h b/src/utils/execmd.h index b3b2db91..24fe27f7 100644 --- a/src/utils/execmd.h +++ b/src/utils/execmd.h @@ -16,7 +16,7 @@ */ #ifndef _EXECMD_H_INCLUDED_ #define _EXECMD_H_INCLUDED_ -/* @(#$Id: execmd.h,v 1.11 2006-12-14 13:53:43 dockes Exp $ (C) 2004 J.F.Dockes */ +/* @(#$Id: execmd.h,v 1.12 2007-05-21 13:30:22 dockes Exp $ (C) 2004 J.F.Dockes */ #include #include @@ -55,6 +55,10 @@ class ExecCmdProvide { * Output from the command is normally returned in a single string, but a * callback can be set to be called whenever new data arrives, in which case * it is permissible to consume the data and erase the string. + * + * Note that SIGPIPE should be ignored and SIGCLD blocked when calling doexec, + * else things might fail randomly. (This is not done inside the class because + * of concerns with multithreaded programs). * */ class ExecCmd {