make sure signals are only handled by the main thread. Fix bus error on rclmon exit (double delete)

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 <stdio.h>
 #include <signal.h>
 #include <locale.h>
+#include <pthread.h>
 
 #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);
+}

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 <string>
 #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_ */

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<string> modified;
 	list<string> 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;
 }

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;
 }
 

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;
+	deleteZ(dbindexer);
-	dbindexer = 0;
     }
 
     if (!dbindexer)
@@ -199,10 +198,8 @@ static bool createstemdb(RclConfig *config, const string &lang)
 
 static void cleanup()
 {
-    delete confindexer;
+    deleteZ(confindexer);
-    confindexer = 0;
+    deleteZ(dbindexer);
-    delete dbindexer;
-    dbindexer = 0;
 }
 
 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))
+	bool monret = startMonitor(config, opts);
-	    exit(0);
+	MONDEB(("Monitor returned %d, exiting\n", monret));
-	exit(1);
+	exit(monret == false);
 #endif // MONITOR
 
 #ifdef RCL_USE_ASPELL

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 = new ConfIndexer(myconf, &idxthread);
-    idxthread.indexer = ix;
     idxthread.loglevel = DebugLog::getdbl()->getlevel();
     idxthread.start();
 }

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;
+    deleteZ(rcldb);
-    rcldb = 0;
+    deleteZ(rclconfig);
-    delete rclconfig;
-    rclconfig = 0;
 #ifdef RCL_USE_ASPELL
-    delete aspell;
+    deleteZ(aspell);
-    aspell = 0;
 #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

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<string>& terms);

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 <fcntl.h>
 #include <errno.h>
 #include <signal.h>
+
 #ifdef PUTENV_ARG_NOT_CONST
 #include <string.h>
 #endif
@@ -178,14 +179,6 @@ int ExecCmd::doexec(const string &cmd, const list<string>& 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<string>& 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;
 

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 <string>
 #include <list>
@@ -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 {