From 69b491feb3c4a7a33cc1d3cc13e36055b357c72f Mon Sep 17 00:00:00 2001
From: Jean-Francois Dockes <jfd@recoll.org>
Date: Wed, 2 Mar 2011 16:20:25 +0100
Subject: [PATCH] recoll gui indexing: make execmd thread-safe. This plus the
 previous change about accessing the global config should fix the crashes
 observed when changing the configuration throgh the gui while the indexing
 thread is running

---
 src/qtgui/confgui/confguiindex.cpp |  7 ++-----
 src/qtgui/idxthread.cpp            |  5 +++--
 src/qtgui/main.cpp                 |  5 -----
 src/qtgui/rclmain_w.cpp            |  1 +
 src/utils/execmd.cpp               | 27 +++++++++++++++++++++++++++
 5 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/qtgui/confgui/confguiindex.cpp b/src/qtgui/confgui/confguiindex.cpp
index 48826732..253fdb6f 100644
--- a/src/qtgui/confgui/confguiindex.cpp
+++ b/src/qtgui/confgui/confguiindex.cpp
@@ -67,13 +67,11 @@ void ConfIndexW::acceptChanges()
 	QMessageBox::critical(0, "Recoll",  
 			      tr("Can't write configuration file"));
     }
-    // Delete local copy
+    // Delete local copy and update the main one from the file
     delete m_conf;
     m_conf = 0;
-    // Update in-memory config
     m_rclconf->updateMainConfig();
 
-    QTimer::singleShot(0, this, SLOT(reloadPanels()));
     if (startIndexingAfterConfig) {
 	startIndexingAfterConfig = 0;
 	start_indexing(true);
@@ -84,10 +82,9 @@ void ConfIndexW::acceptChanges()
 void ConfIndexW::rejectChanges()
 {
     LOGDEB(("ConfIndexW::rejectChanges()\n"));
-    // Discard local changes, and make new copy
+    // Discard local changes.
     delete m_conf;
     m_conf = 0;
-    QTimer::singleShot(0, this, SLOT(reloadPanels()));
     hide();
 }
 
diff --git a/src/qtgui/idxthread.cpp b/src/qtgui/idxthread.cpp
index 497ebdee..99f00981 100644
--- a/src/qtgui/idxthread.cpp
+++ b/src/qtgui/idxthread.cpp
@@ -74,8 +74,8 @@ void IdxThread::run()
 
 	    m_interrupted = false;
 	    indexingstatus = IDXTS_NULL;
-	    // We have to make a copy of the config (setKeydir changes
-	    // it during indexing)
+	    // We make a private snapshot of the config: setKeydir changes
+	    // it during indexing and it may be updated by the main thread.
 	    RclConfig *myconf = new RclConfig(*cnf);
 	    int loglevel;
 	    myconf->setKeyDir("");
@@ -97,6 +97,7 @@ void IdxThread::run()
 		    indexingstatus = IDXTS_ERROR;
 		    indexingReason = "Indexing failed: " + indexer->getReason();
 		}
+		delete myconf;
 		pidfile.close();
 		delete indexer;
 	    }
diff --git a/src/qtgui/main.cpp b/src/qtgui/main.cpp
index b05ac7d7..e588c182 100644
--- a/src/qtgui/main.cpp
+++ b/src/qtgui/main.cpp
@@ -56,11 +56,6 @@ Rcl::Db *rcldb;
 Aspell *aspell;
 #endif
 
-RclConfig* RclConfig::getMainConfig()
-{
-    return rclconfig;
-}
-
 RclDynConf *g_dynconf;
 int recollNeedsExit;
 int startIndexingAfterConfig;
diff --git a/src/qtgui/rclmain_w.cpp b/src/qtgui/rclmain_w.cpp
index f78b48e7..4ffa8701 100644
--- a/src/qtgui/rclmain_w.cpp
+++ b/src/qtgui/rclmain_w.cpp
@@ -668,6 +668,7 @@ void RclMain::showIndexConfig()
     } else {
 	// Close and reopen, in hope that makes us visible...
 	indexConfig->close();
+	indexConfig->reloadPanels();
     }
     indexConfig->show();
 }
diff --git a/src/utils/execmd.cpp b/src/utils/execmd.cpp
index a36be31a..088367e8 100644
--- a/src/utils/execmd.cpp
+++ b/src/utils/execmd.cpp
@@ -315,6 +315,31 @@ private:
     ExecCmdAdvise *m_advise;
 };
 
+
+// The netcon selectloop that doexec() uses for reading/writing would
+// be complicated to render thread-safe. Use locking to ensure only
+// one thread in there
+class ExecLocking {
+public:
+    pthread_mutex_t m_mutex;
+    ExecLocking() 
+    {
+	pthread_mutex_init(&m_mutex, 0);
+    }
+};
+ExecLocking o_lock;
+class ExecLocker {
+public:
+    ExecLocker()
+    {
+	pthread_mutex_lock(&o_lock.m_mutex);
+    }
+    ~ExecLocker()
+    {
+	pthread_mutex_unlock(&o_lock.m_mutex);
+    }
+};
+
 int ExecCmd::doexec(const string &cmd, const list<string>& args,
 		    const string *input, string *output)
 {
@@ -324,6 +349,8 @@ int ExecCmd::doexec(const string &cmd, const list<string>& args,
 
     // Cleanup in case we return early
     ExecCmdRsrc e(this);
+    // Only one thread allowed in here...
+    ExecLocker locker;
 
     int ret = 0;
     if (input || output) {