Fix pure virtual method called error when running CI tests (#1930)

* Fix `pure virtual method called` error when running CI tests

Unsafe to call methods from virtual CThread destructor.
Add `terminate()` method to all derived classes and call explicitly _before_ destroying the object.

* Added debug statements to confirm thread sequencing correct.

* Fix keyboard thread race condition.

Small test applications can execute and finish before the keyboard thread has started running.
Adding a small delay and an additional check handles this situation and avoids calling the keyb_XX functions.

This doesn't guarantee that `keyb_raw()` isn't called during cleanup as the race condition still exists.
This isn't a problem in itself, except for the call to `atexit()` which results in undefined behaviour
and in practice, a deadlock. It's not required as the keyboard thread calls `keyb_restore()` when it's done.

.travis/build.sh

@@ -68,7 +68,7 @@ if [ "$TRAVIS_BUILD_STAGE_NAME" == "Test" ]; then
 
 	# Build and run tests
 	export SMING_TARGET_OPTIONS='--flashfile=$(FLASH_BIN) --flashsize=$(SPI_SIZE)'
-	$MAKE_PARALLEL tests || true
+	$MAKE_PARALLEL tests
 
 	# Build the documentation
 	mv $SMING_PROJECTS_DIR/samples ..

Sming/Arch/Host/Components/driver/hw_timer.cpp

@@ -34,10 +34,11 @@ class CTimerThread : public CThread
 		execute();
 	}
 
-	~CTimerThread()
+	void terminate()
 	{
 		state = terminating;
 		sem.post();
+		join();
 	}
 
 	void attach_interrupt(hw_timer_source_type_t source_type, hw_timer_callback_t callback, void* arg)
@@ -138,8 +139,6 @@ class CTimerThread : public CThread
 	bool auto_load = false;
 };
 
-static CTimerThread timer1("Timer1");
-
 void* CTimerThread::thread_routine()
 {
 #ifdef __WIN32
@@ -208,6 +207,13 @@ void* CTimerThread::thread_routine()
 	return nullptr;
 }
 
+static CTimerThread timer1("Timer1");
+
+void hw_timer_cleanup()
+{
+	timer1.terminate();
+}
+
 void hw_timer1_attach_interrupt(hw_timer_source_type_t source_type, hw_timer_callback_t callback, void* arg)
 {
 	timer1.attach_interrupt(source_type, callback, arg);

Sming/Arch/Host/Components/driver/include/driver/hw_timer.h

@@ -83,6 +83,8 @@ inline void hw_timer_init(void)
 {
 }
 
+void hw_timer_cleanup();
+
 #ifdef __cplusplus
 }
 #endif

Sming/Arch/Host/Components/driver/uart_server.cpp

@@ -36,9 +36,10 @@ class KeyboardThread : public CThread
 	{
 	}
 
-	~KeyboardThread()
+	void terminate()
 	{
 		done = true;
+		join();
 	}
 
 protected:
@@ -50,6 +51,12 @@ class KeyboardThread : public CThread
 
 void* KeyboardThread::thread_routine()
 {
+	// Small applications can complete before we even get here!
+	msleep(50);
+	if(done) {
+		return nullptr;
+	}
+
 	keyb_raw();
 	while(!done) {
 		int c = getkey();
@@ -84,6 +91,17 @@ void* KeyboardThread::thread_routine()
 
 static KeyboardThread* keyboardThread;
 
+static void destroyKeyboardThread()
+{
+	if(keyboardThread == nullptr) {
+		return;
+	}
+
+	keyboardThread->terminate();
+	delete keyboardThread;
+	keyboardThread = nullptr;
+}
+
 static void onUart0Notify(uart_t* uart, uart_notify_code_t code)
 {
 	switch(code) {
@@ -105,12 +123,9 @@ static void onUart0Notify(uart_t* uart, uart_notify_code_t code)
 		}
 		break;
 
-	case UART_NOTIFY_BEFORE_CLOSE: {
-		auto thread = keyboardThread;
-		keyboardThread = nullptr;
-		delete thread;
+	case UART_NOTIFY_BEFORE_CLOSE:
+		destroyKeyboardThread();
 		break;
-	}
 
 	default:; // ignore
 	}
@@ -150,16 +165,24 @@ void CUartServer::startup(const UartServerConfig& config)
 
 void CUartServer::shutdown()
 {
+	destroyKeyboardThread();
+
 	for(unsigned i = 0; i < UART_COUNT; ++i) {
 		auto& server = uartServers[i];
+		if(server == nullptr) {
+			continue;
+		}
+
+		server->terminate();
 		delete server;
 		server = nullptr;
 	}
 }
 
-CUartServer::~CUartServer()
+void CUartServer::terminate()
 {
 	close();
+	join();
 	hostmsg("UART%u server destroyed", uart_nr);
 }
 

Sming/Arch/Host/Components/driver/uart_server.h

@@ -56,7 +56,7 @@ class CUartServer : public CThread, public CServerSocket
 	{
 	}
 
-	~CUartServer();
+	void terminate();
 
 protected:
 	void onNotify(uart_t* uart, uart_notify_code_t code);

Sming/Arch/Host/Components/hostlib/keyb.cpp

@@ -246,7 +246,6 @@ void keyb_raw()
 		// make stdin non-blocking
 		g_orig_flags = fcntl(0, F_GETFL);
 		tcgetattr(STDIN_FILENO, &g_orig_attr);
-		atexit(keyb_restore);
 	}
 
 	// make stdin non-blocking

Sming/Arch/Host/Components/hostlib/startup.cpp

@@ -44,6 +44,7 @@ extern void host_init_bootloader();
 
 static void cleanup()
 {
+	hw_timer_cleanup();
 	host_flashmem_cleanup();
 	CUartServer::shutdown();
 	sockets_finalise();

Sming/Arch/Host/Components/hostlib/threads.h

@@ -20,9 +20,16 @@
 #pragma once
 
 #include "hostlib.h"
+#include "hostmsg.h"
 #include <pthread.h>
 #include <semaphore.h>
 
+#if defined(DEBUG_VERBOSE_LEVEL) && (DEBUG_VERBOSE_LEVEL == 3)
+#define HOST_THREAD_DEBUG(fmt, ...) host_printf(fmt "\n", ##__VA_ARGS__)
+#else
+#define HOST_THREAD_DEBUG(fmt, ...)
+#endif
+
 class CMutex;
 
 class CThread
@@ -39,7 +46,7 @@ class CThread
 
 	virtual ~CThread()
 	{
-		join();
+		HOST_THREAD_DEBUG("Thread '%s' destroyed", name);
 	}
 
 	bool execute()
@@ -60,6 +67,7 @@ class CThread
 	void join()
 	{
 		pthread_join(m_thread, nullptr);
+		HOST_THREAD_DEBUG("Thread '%s' complete", name);
 	}
 
 	/*
@@ -92,6 +100,7 @@ class CThread
 	static void* thread_start(void* param)
 	{
 		auto thread = static_cast<CThread*>(param);
+		HOST_THREAD_DEBUG("Thread '%s' running", thread->name);
 		return thread->thread_routine();
 	}