Merge pull request #1174 from DarthGandalf/thread

Rework signal handling and use C++11 threads.

include/znc/Threads.h

@@ -29,179 +29,27 @@
 #include <cstring>
 #include <list>
 #include <pthread.h>
+#include <mutex>
+#include <condition_variable>
 
 /**
  * This class represents a non-recursive mutex. Only a single thread may own the
  * mutex at any point in time.
  */
-class CMutex {
-public:
-	friend class CConditionVariable;
-
-	CMutex() : m_mutex() {
-		int i = pthread_mutex_init(&m_mutex, nullptr);
-		if (i) {
-			CUtils::PrintError("Can't initialize mutex: " + CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	~CMutex() {
-		int i = pthread_mutex_destroy(&m_mutex);
-		if (i) {
-			CUtils::PrintError("Can't destroy mutex: " + CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	void lock() {
-		int i = pthread_mutex_lock(&m_mutex);
-		if (i) {
-			CUtils::PrintError("Can't lock mutex: " + CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	void unlock() {
-		int i = pthread_mutex_unlock(&m_mutex);
-		if (i) {
-			CUtils::PrintError("Can't unlock mutex: " + CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-private:
-	// Undefined copy constructor and assignment operator
-	CMutex(const CMutex&);
-	CMutex& operator=(const CMutex&);
-
-	pthread_mutex_t m_mutex;
-};
+using CMutex = std::mutex;
 
 /**
  * A mutex locker should always be used as an automatic variable. This
  * class makes sure that the mutex is unlocked when this class is destructed.
  * For example, this makes it easier to make code exception-safe.
  */
-class CMutexLocker {
-public:
-	CMutexLocker(CMutex& mutex, bool initiallyLocked = true)
-			: m_mutex(mutex), m_locked(false) {
-		if (initiallyLocked)
-			lock();
-	}
-
-	~CMutexLocker() {
-		if (m_locked)
-			unlock();
-	}
-
-	void lock() {
-		assert(!m_locked);
-		m_mutex.lock();
-		m_locked = true;
-	}
-
-	void unlock() {
-		assert(m_locked);
-		m_locked = false;
-		m_mutex.unlock();
-	}
-
-private:
-	// Undefined copy constructor and assignment operator
-	CMutexLocker(const CMutexLocker&);
-	CMutexLocker& operator=(const CMutexLocker&);
-
-	CMutex &m_mutex;
-	bool m_locked;
-};
+using CMutexLocker = std::unique_lock<std::mutex>;
 
 /**
  * A condition variable makes it possible for threads to wait until some
  * condition is reached at which point the thread can wake up again.
  */
-class CConditionVariable {
-public:
-	CConditionVariable() : m_cond() {
-		int i = pthread_cond_init(&m_cond, nullptr);
-		if (i) {
-			CUtils::PrintError("Can't initialize condition variable: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	~CConditionVariable() {
-		int i = pthread_cond_destroy(&m_cond);
-		if (i) {
-			CUtils::PrintError("Can't destroy condition variable: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	void wait(CMutex& mutex) {
-		int i = pthread_cond_wait(&m_cond, &mutex.m_mutex);
-		if (i) {
-			CUtils::PrintError("Can't wait on condition variable: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	void signal() {
-		int i = pthread_cond_signal(&m_cond);
-		if (i) {
-			CUtils::PrintError("Can't signal condition variable: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-	void broadcast() {
-		int i = pthread_cond_broadcast(&m_cond);
-		if (i) {
-			CUtils::PrintError("Can't broadcast condition variable: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-private:
-	// Undefined copy constructor and assignment operator
-	CConditionVariable(const CConditionVariable&);
-	CConditionVariable& operator=(const CConditionVariable&);
-
-	pthread_cond_t m_cond;
-};
-
-class CThread {
-public:
-	typedef void *threadRoutine(void *);
-	static void startThread(threadRoutine *func, void *arg) {
-		pthread_t thr;
-		sigset_t old_sigmask, sigmask;
-
-		/* Block all signals. The thread will inherit our signal mask
-		 * and thus won't ever try to handle signals.
-		 */
-		int i = sigfillset(&sigmask);
-		i |= pthread_sigmask(SIG_SETMASK, &sigmask, &old_sigmask);
-		i |= pthread_create(&thr, nullptr, func, arg);
-		i |= pthread_sigmask(SIG_SETMASK, &old_sigmask, nullptr);
-		i |= pthread_detach(thr);
-		if (i) {
-			CUtils::PrintError("Can't start new thread: "
-					+ CString(strerror(errno)));
-			exit(1);
-		}
-	}
-
-private:
-	// Undefined constructor
-	CThread();
-};
+using CConditionVariable = std::condition_variable_any;
 
 /**
  * A job is a task which should run without blocking the main thread. You do
@@ -292,11 +140,6 @@ private:
 	void finishJob(CJob *) const;
 
 	void threadFunc();
-	static void *threadPoolFunc(void *arg) {
-		CThreadPool &pool = *reinterpret_cast<CThreadPool *>(arg);
-		pool.threadFunc();
-		return nullptr;
-	}
 
 	// mutex protecting all of these members
 	CMutex m_mutex;

include/znc/znc.h

@@ -22,6 +22,7 @@
 #include <znc/Modules.h>
 #include <znc/Socket.h>
 #include <znc/Listener.h>
+#include <mutex>
 #include <map>
 #include <list>
 
@@ -100,7 +101,7 @@ public:
 	void AuthUser(std::shared_ptr<CAuthBase> AuthClass);
 
 	// Setters
-	void SetConfigState(enum ConfigState e) { m_eConfigState = e; }
+	void SetConfigState(enum ConfigState e) { std::lock_guard<std::mutex> guard(m_mutexConfigState); m_eConfigState = e; }
 	void SetSkinName(const CString& s) { m_sSkinName = s; }
 	void SetStatusPrefix(const CString& s) { m_sStatusPrefix = (s.empty()) ? "*" : s; }
 	void SetMaxBufferSize(unsigned int i) { m_uiMaxBufferSize = i; }
@@ -115,7 +116,7 @@ public:
 	// !Setters
 
 	// Getters
-	enum ConfigState GetConfigState() const { return m_eConfigState; }
+	enum ConfigState GetConfigState() { std::lock_guard<std::mutex> guard(m_mutexConfigState); return m_eConfigState; }
 	CSockManager& GetManager() { return m_Manager; }
 	const CSockManager& GetManager() const { return m_Manager; }
 	CModules& GetModules() { return *m_pModules; }
@@ -222,7 +223,9 @@ protected:
 	time_t                 m_TimeStarted;
 
 	enum ConfigState       m_eConfigState;
-	std::vector<CListener*>     m_vpListeners;
+	std::mutex             m_mutexConfigState;
+
+	std::vector<CListener*>  m_vpListeners;
 	std::map<CString,CUser*> m_msUsers;
 	std::map<CString,CUser*> m_msDelUsers;
 	CSockManager           m_Manager;

src/Threads.cpp

@@ -20,6 +20,7 @@
 
 #include <znc/ZNCDebug.h>
 #include <algorithm>
+#include <thread>
 
 /* Just an arbitrary limit for the number of idle threads */
 static const size_t MAX_IDLE_THREADS = 3;
@@ -49,7 +50,7 @@ void CThreadPool::jobDone(CJob* job) {
 
 	if (oldState == CJob::CANCELLED) {
 		// Signal the main thread that cancellation is done
-		m_cancellationCond.signal();
+		m_cancellationCond.notify_one();
 		return;
 	}
 
@@ -88,7 +89,7 @@ CThreadPool::~CThreadPool() {
 	m_done = true;
 
 	while (m_num_threads > 0) {
-		m_cond.broadcast();
+		m_cond.notify_all();
 		m_exit_cond.wait(m_mutex);
 	}
 }
@@ -133,7 +134,7 @@ void CThreadPool::threadFunc() {
 	m_num_idle--;
 
 	if (m_num_threads == 0 && m_done)
-		m_exit_cond.signal();
+		m_exit_cond.notify_one();
 }
 
 void CThreadPool::addJob(CJob *job) {
@@ -142,7 +143,7 @@ void CThreadPool::addJob(CJob *job) {
 
 	// Do we already have a thread which can handle this job?
 	if (m_num_idle > 0) {
-		m_cond.signal();
+		m_cond.notify_one();
 		return;
 	}
 
@@ -153,7 +154,9 @@ void CThreadPool::addJob(CJob *job) {
 
 	// Start a new thread for our pool
 	m_num_threads++;
-	CThread::startThread(threadPoolFunc, this);
+	std::thread([this]() {
+		threadFunc();
+	}).detach();
 }
 
 void CThreadPool::cancelJob(CJob *job) {

src/main.cpp

@@ -22,6 +22,7 @@
 #include <znc/Threads.h>
 #include <openssl/crypto.h>
 #include <memory>
+#include <thread>
 
 static std::vector<std::unique_ptr<CMutex> > lock_cs;
 
@@ -130,24 +131,64 @@ static void GenerateHelp(const char *appname) {
 	CUtils::PrintMessage("\t-d, --datadir      Set a different ZNC repository (default is ~/.znc)");
 }
 
-static void die(int sig) {
-	signal(SIGPIPE, SIG_DFL);
-	CZNC::Get().SetConfigState(CZNC::ECONFIG_NEED_QUIT);
-}
-
-static void signalHandler(int sig) {
-	switch (sig) {
-	case SIGHUP:
-		CZNC::Get().SetConfigState(CZNC::ECONFIG_NEED_REHASH);
-		break;
-	case SIGUSR1:
-		CZNC::Get().SetConfigState(CZNC::ECONFIG_NEED_VERBOSE_WRITE);
-		break;
-	default:
-		// WTF? Signal handler called for a signal it doesn't know?
-		abort();
+class CSignalHandler {
+public:
+	CSignalHandler(CZNC* pZNC) {
+		sigset_t signals;
+		sigfillset(&signals);
+		pthread_sigmask(SIG_SETMASK, &signals, nullptr);
+		m_thread = std::thread([=]() {
+			HandleSignals(pZNC);
+		});
 	}
-}
+	~CSignalHandler() {
+		pthread_cancel(m_thread.native_handle());
+		m_thread.join();
+	}
+private:
+	void HandleSignals(CZNC* pZNC) {
+		sigset_t signals;
+		sigemptyset(&signals);
+		sigaddset(&signals, SIGHUP);
+		sigaddset(&signals, SIGUSR1);
+		sigaddset(&signals, SIGINT);
+		sigaddset(&signals, SIGQUIT);
+		sigaddset(&signals, SIGTERM);
+		sigaddset(&signals, SIGPIPE);
+		// Handle only these signals specially; the rest will have their default action, but in this thread
+		pthread_sigmask(SIG_SETMASK, &signals, nullptr);
+		while (true) {
+			int sig;
+			pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, nullptr);
+			if (sigwait(&signals, &sig) == -1) continue;
+			pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, nullptr);
+			switch (sig) {
+				case SIGHUP:
+					pZNC->SetConfigState(CZNC::ECONFIG_NEED_REHASH);
+					break;
+				case SIGUSR1:
+					pZNC->SetConfigState(CZNC::ECONFIG_NEED_VERBOSE_WRITE);
+					break;
+				case SIGINT:
+				case SIGQUIT:
+				case SIGTERM:
+					pZNC->SetConfigState(CZNC::ECONFIG_NEED_QUIT);
+					// Reset handler to default by:
+					// * not blocking it
+					// * not waiting for it
+					// So, if ^C is pressed, but for some reason it didn't work, second ^C will kill the process for sure.
+					sigdelset(&signals, sig);
+					pthread_sigmask(SIG_SETMASK, &signals, nullptr);
+					break;
+				case SIGPIPE:
+				default:
+					break;
+			}
+		}
+	}
+
+	std::thread m_thread;
+};
 
 static bool isRoot() {
 	// User root? If one of these were root, we could switch the others to root, too
@@ -385,24 +426,8 @@ int main(int argc, char** argv) {
 		// controlling terminal). We are independent!
 	}
 
-	struct sigaction sa;
-	sa.sa_flags = 0;
-	sigemptyset(&sa.sa_mask);
-
-	sa.sa_handler = SIG_IGN;
-	sigaction(SIGPIPE, &sa, (struct sigaction*) nullptr);
-
-	sa.sa_handler = signalHandler;
-	sigaction(SIGHUP,  &sa, (struct sigaction*) nullptr);
-	sigaction(SIGUSR1, &sa, (struct sigaction*) nullptr);
-
-	// Once this signal is caught, the signal handler is reset
-	// to SIG_DFL. This avoids endless loop with signals.
-	sa.sa_flags = SA_RESETHAND;
-	sa.sa_handler = die;
-	sigaction(SIGINT,  &sa, (struct sigaction*) nullptr);
-	sigaction(SIGQUIT, &sa, (struct sigaction*) nullptr);
-	sigaction(SIGTERM, &sa, (struct sigaction*) nullptr);
+	// Handle all signals in separate thread
+	std::unique_ptr<CSignalHandler> SignalHandler(new CSignalHandler(pZNC));
 
 	int iRet = 0;
 
@@ -436,6 +461,7 @@ int main(int argc, char** argv) {
 				// The above code adds 3 entries to args tops
 				// which means the array should be big enough
 
+				SignalHandler.reset();
 				CZNC::DestroyInstance();
 				execvp(args[0], args);
 				CUtils::PrintError("Unable to restart ZNC [" + CString(strerror(errno)) + "]");
@@ -445,6 +471,7 @@ int main(int argc, char** argv) {
 		}
 	}
 
+	SignalHandler.reset();
 	CZNC::DestroyInstance();
 
 	CUtils::PrintMessage("Exiting");

test/ThreadTest.cpp

@@ -40,14 +40,14 @@ public:
 
 		// and signal it to exit
 		m_bThreadDone = true;
-		m_CV.broadcast();
+		m_CV.notify_all();
 	}
 
 	virtual void runThread() {
 		CMutexLocker locker(m_Mutex);
 		// We are running
 		m_bThreadReady = true;
-		m_CV.broadcast();
+		m_CV.notify_all();
 
 		// wait for our exit signal
 		while (!m_bThreadDone)
@@ -97,7 +97,7 @@ public:
 		m_Mutex.lock();
 		// We are running, tell the main thread
 		m_bThreadReady = true;
-		m_CVThreadReady.broadcast();
+		m_CVThreadReady.notify_all();
 		// Have to unlock here so that wait() can get the mutex
 		m_Mutex.unlock();