cIsThread cleanup

+ Semi-gracefully handle unexpected exceptions
* No-one cared about the return values, remove them

Tools/MCADefrag/MCADefrag.cpp

@@ -86,7 +86,7 @@ void cMCADefrag::Run(void)
 	// Wait for all the threads to finish:
 	while (!m_Threads.empty())
 	{
-		m_Threads.front()->Wait();
+		m_Threads.front()->Stop();
 		delete m_Threads.front();
 		m_Threads.pop_front();
 	}

src/DeadlockDetect.cpp

@@ -51,18 +51,18 @@ cDeadlockDetect::~cDeadlockDetect()
 
 
 
-bool cDeadlockDetect::Start(int a_IntervalSec)
+void cDeadlockDetect::Start(int a_IntervalSec)
 {
 	m_IntervalSec = a_IntervalSec;
 
 	// Read the initial world data:
 	cRoot::Get()->ForEachWorld([=](cWorld & a_World)
-		{
+	{
-			SetWorldAge(a_World.GetName(), a_World.GetWorldAge());
+		SetWorldAge(a_World.GetName(), a_World.GetWorldAge());
-			return false;
+		return false;
-		}
+	});
-	);
+
-	return Super::Start();
+	Super::Start();
 }
 
 

src/DeadlockDetect.h

@@ -31,7 +31,7 @@ public:
 	virtual ~cDeadlockDetect() override;
 
 	/** Starts the detection. Hides cIsThread's Start, because we need some initialization */
-	bool Start(int a_IntervalSec);
+	void Start(int a_IntervalSec);
 
 	/** Adds the critical section for tracking.
 	Tracked CSs are listed, together with ownership details, when a deadlock is detected.

src/OSSupport/IsThread.cpp

@@ -32,13 +32,77 @@ cIsThread::~cIsThread()
 
 
 
-void cIsThread::DoExecute(void)
+void cIsThread::Start(void)
+{
+	// Initialize the thread:
+	m_Thread = std::thread(&cIsThread::Entrypoint, this);
+
+	// Notify the thread that initialization is complete and it can run its code safely:
+	m_Initialisation.Set();
+}
+
+
+
+
+
+void cIsThread::Stop(void)
+{
+	m_ShouldTerminate = true;
+	{
+		LOGD("Waiting for the %s thread to finish", m_ThreadName.c_str());
+		if (m_Thread.joinable())
+		{
+			m_Thread.join();
+		}
+		LOGD("The %s thread finished", m_ThreadName.c_str());
+	}
+	m_ShouldTerminate = false;
+}
+
+
+
+
+
+void cIsThread::Entrypoint(void)
+{
+	// Apply thread naming:
+	SetThreadName();
+
+	// Wait for initialisation:
+	m_Initialisation.Wait();
+
+	try
+	{
+		Execute();
+	}
+	catch (const std::exception & Oops)
+	{
+		LOGERROR("Thread %s faulted with standard exception: %s", m_ThreadName.c_str(), Oops.what());
+		std::abort();
+	}
+	catch (...)
+	{
+		LOGERROR("Thread %s faulted with unknown exception!", m_ThreadName.c_str());
+		std::abort();
+	}
+}
+
+
+
+
+
+void cIsThread::SetThreadName() const
 {
 #if defined(_MSC_VER) && !defined(NDEBUG)
 	/* Sets the name of this thread.
 	(When in MSVC, the debugger provides "thread naming" by catching special exceptions)
 	Code adapted from MSDN: https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx */
 
+	if (m_ThreadName.empty())
+	{
+		return;
+	}
+
 #pragma pack(push, 8)
 	struct THREADNAME_INFO
 	{
@@ -49,80 +113,15 @@ void cIsThread::DoExecute(void)
 	};
 #pragma pack(pop)
 
-	if (!m_ThreadName.empty())
+	const DWORD NAME_EXCEPTION = 0x406D1388;
-	{
+	const THREADNAME_INFO Name = { 0x1000, m_ThreadName.c_str(), -1, 0 };
-		const DWORD NAME_EXCEPTION = 0x406D1388;
-		const THREADNAME_INFO Name = { 0x1000, m_ThreadName.c_str(), -1, 0 };
 
-		__try
+	__try
-		{
+	{
-			RaiseException(NAME_EXCEPTION, 0, sizeof(Name) / sizeof(ULONG_PTR), reinterpret_cast<const ULONG_PTR *>(&Name));
+		RaiseException(NAME_EXCEPTION, 0, sizeof(Name) / sizeof(ULONG_PTR), reinterpret_cast<const ULONG_PTR *>(&Name));
-		}
+	}
-		__except (EXCEPTION_EXECUTE_HANDLER)
+	__except (EXCEPTION_EXECUTE_HANDLER)
-		{
+	{
-		}
 	}
 #endif
-
-	m_evtStart.Wait();
-	Execute();
-}
-
-
-
-
-
-bool cIsThread::Start(void)
-{
-	try
-	{
-		// Initialize the thread:
-		m_Thread = std::thread(&cIsThread::DoExecute, this);
-
-		// Notify the thread that initialization is complete and it can run its code safely:
-		m_evtStart.Set();
-
-		return true;
-	}
-	catch (const std::system_error & a_Exception)
-	{
-		LOGERROR("cIsThread::Start error %i: could not construct thread %s; %s", a_Exception.code().value(), m_ThreadName.c_str(), a_Exception.code().message().c_str());
-		return false;
-	}
-}
-
-
-
-
-
-void cIsThread::Stop(void)
-{
-	m_ShouldTerminate = true;
-	Wait();
-	m_ShouldTerminate = false;
-}
-
-
-
-
-
-bool cIsThread::Wait(void)
-{
-	LOGD("Waiting for the %s thread to finish", m_ThreadName.c_str());
-	if (m_Thread.joinable())
-	{
-		try
-		{
-			m_Thread.join();
-			return true;
-		}
-		catch (const std::system_error & a_Exception)
-		{
-			LOGERROR("%s error %i: could not join the %s thread; %s", __FUNCTION__, a_Exception.code().value(), m_ThreadName.c_str(), a_Exception.code().message().c_str());
-			return false;
-		}
-	}
-
-	LOGD("The %s thread finished", m_ThreadName.c_str());
-	return true;
 }

src/OSSupport/IsThread.h

@@ -2,11 +2,10 @@
 // IsThread.h
 
 // Interfaces to the cIsThread class representing an OS-independent wrapper for a class that implements a thread.
-// This class will eventually suupersede the old cThread class
 
 /*
 Usage:
-To have a new thread, declare a class descending from cIsClass.
+To have a new thread, declare a class descending from cIsThread.
 Then override its Execute() method to provide your thread processing.
 In the descending class' constructor call the Start() method to start the thread once you're finished with initialization.
 */
@@ -23,46 +22,44 @@ In the descending class' constructor call the Start() method to start the thread
 
 class cIsThread
 {
+public:
+
+	cIsThread(AString && a_ThreadName);
+	virtual ~cIsThread();
+
+	/** Starts the thread; returns without waiting for the actual start. */
+	void Start(void);
+
+	/** Signals the thread to terminate and waits until it's finished. */
+	void Stop(void);
+
+	/** Returns true if the thread calling this function is the thread contained within this object. */
+	bool IsCurrentThread(void) const { return std::this_thread::get_id() == m_Thread.get_id(); }
+
 protected:
-	/** This is the main thread entrypoint.
+
-	This function, overloaded by the descendants, is called in the new thread. */
+	/** This function, overloaded by the descendants, is called in the new thread. */
 	virtual void Execute(void) = 0;
 
 	/** The overriden Execute() method should check this value periodically and terminate if this is true. */
 	std::atomic<bool> m_ShouldTerminate;
 
-public:
-	cIsThread(AString && a_ThreadName);
-	virtual ~cIsThread();
-
-	/** Starts the thread; returns without waiting for the actual start. */
-	bool Start(void);
-
-	/** Signals the thread to terminate and waits until it's finished. */
-	void Stop(void);
-
-	/** Waits for the thread to finish. Doesn't signalize the ShouldTerminate flag. */
-	bool Wait(void);
-
-	/** Returns true if the thread calling this function is the thread contained within this object. */
-	bool IsCurrentThread(void) const { return std::this_thread::get_id() == m_Thread.get_id(); }
-
 private:
 
-	/** The name of the thread, used to aid debugging in IDEs which support named threads */
-	AString m_ThreadName;
-
 	/** The thread object which holds the created thread for later manipulation */
 	std::thread m_Thread;
 
+	/** The name of the thread, used to aid debugging in IDEs which support named threads */
+	AString m_ThreadName;
+
 	/** The event that is used to wait with the thread's execution until the thread object is fully initialized.
 	This prevents the IsCurrentThread() call to fail because of a race-condition where the thread starts before m_Thread has been fully assigned. */
-	cEvent m_evtStart;
+	cEvent m_Initialisation;
 
-	/** Wrapper for Execute() that waits for the initialization event, to prevent race conditions in thread initialization. */
+	/** This is the main thread entrypoint.
-	void DoExecute(void);
+	Wrapper for Execute() that waits for the initialization event, to prevent race conditions in thread initialization. */
+	void Entrypoint(void);
+
+	/** Sets the name of the current thread to be the name provided in m_ThreadName. */
+	void SetThreadName() const;
 } ;
-
-
-
-

src/Server.cpp

@@ -411,7 +411,8 @@ bool cServer::Start(void)
 		LOGERROR("Couldn't open any ports. Aborting the server");
 		return false;
 	}
-	return m_TickThread.Start();
+	m_TickThread.Start();
+	return true;
 }