pica-client: move SkyNet instance to the separate thread

5 files changed, 223 insertions, 30 deletions

diff --git a/pica-client/main.cpp b/pica-client/main.cpp
index e4b7ca9..0fae44c 100644
--- a/pica-client/main.cpp
+++ b/pica-client/main.cpp
@@ -17,6 +17,7 @@
 //#include <QtGui/QApplication>
 #include <QCoreApplication>
 #include <QApplication>
+#include <QThread>
 #include "mainwindow.h"
 #include "accountswindow.h"
 #include "globals.h"
@@ -549,9 +550,56 @@ int main(int argc, char *argv[])
 	AudioVideoCallController avc;
 	avctrl = &avc;
 
+	// SkyNet owns all the protocol/network state (PICA_c2n/PICA_c2c
+	// connections, the timer-driven PICA_event_loop() polling, and every
+	// protocol callback dispatch). It's moved off the GUI thread onto its
+	// own network thread so that GUI-thread stalls (blocked X11/graphics
+	// calls, a locked screen suspending the process, etc.) can no longer
+	// starve the network connection and cause disconnects. Its public
+	// methods remain safe to call from any thread - see SkyNet::init() and
+	// the do_*() wrapper pattern in skynet.cpp/h.
+	//
+	// Done last, after every controller above has already connected its
+	// signal/slot listeners to skynet while it was still on the GUI
+	// thread, so there is no window during which a listener could miss a
+	// signal skynet emits from its new thread.
+	QThread networkThread;
+	networkThread.setObjectName("SkyNetThread");
+
+	skynet->moveToThread(&networkThread);
+	QObject::connect(&networkThread, SIGNAL(started()), skynet, SLOT(init()));
+	networkThread.start();
+
 	aw.show();
 
 	a.setQuitOnLastWindowClosed(false);
 
-	return a.exec();
+	int ret = a.exec();
+
+	// skynet's thread affinity needs to move back to the main thread
+	// before it (and everything else declared above) gets destroyed as
+	// main() returns - otherwise QObject's destructor can't clean up its
+	// still-registered timers (event_loop_timer_id, c2c_reconnect_timer_id,
+	// ...) and prints "Timers cannot be stopped from another thread".
+	//
+	// QObject::moveToThread() only succeeds when called from the thread
+	// the object is currently on - it can be pushed out from its own
+	// thread, but not pulled in from a different one. So this can't be a
+	// plain skynet->moveToThread(...) call made from here (the main
+	// thread); it has to run as code executing on the network thread
+	// itself, which is what moveBackToMainThread() is for. Using a
+	// blocking queued call guarantees it has actually completed before
+	// the network thread is stopped below - if it ran after quit()/wait(),
+	// there would be no event loop left on that thread to process it.
+	QMetaObject::invokeMethod(skynet, "moveBackToMainThread", Qt::BlockingQueuedConnection);
+
+	// Now stop and join the network thread before any of the objects
+	// above - skynet in particular - start being destroyed as main()
+	// unwinds. Otherwise the network thread could still be executing
+	// SkyNet code while its destructor (and the destructors of objects it
+	// signals) runs concurrently on this thread.
+	networkThread.quit();
+	networkThread.wait();
+
+	return ret;
 }
diff --git a/pica-client/mainwindow.cpp b/pica-client/mainwindow.cpp
index 2bbecb5..2e7fed6 100644
--- a/pica-client/mainwindow.cpp
+++ b/pica-client/mainwindow.cpp
@@ -26,7 +26,8 @@
 
 MainWindow::MainWindow(QWidget *parent) :
 	QMainWindow(parent),
-	ui(new Ui::MainWindow)
+	ui(new Ui::MainWindow),
+	self_aware_cached(false)
 {
 	ui->setupUi(this);
 
@@ -73,11 +74,13 @@ MainWindow::~MainWindow()
 
 void MainWindow::set_offline()
 {
+	self_aware_cached = false;
 	SetStatus(false);
 }
 
 void MainWindow::set_online()
 {
+	self_aware_cached = true;
 	SetStatus(true);
 }
 
@@ -86,12 +89,11 @@ void MainWindow::status_changed(int index)
 	if (status_change_disable_flag)
 		return;
 
-	if (index == 0 && !skynet->isSelfAware())
+	if (index == 0 && !self_aware_cached)
 	{
-		Accounts::AccountRecord curr_acc = skynet->CurrentAccount();
-		skynet->Join(curr_acc);
+		skynet->Join(Accounts::GetCurrentAccount());
 	}
-	if (1 == index && skynet->isSelfAware())
+	if (1 == index && self_aware_cached)
 		skynet->Exit();
 }
 
diff --git a/pica-client/mainwindow.h b/pica-client/mainwindow.h
index 6360723..378cdf0 100644
--- a/pica-client/mainwindow.h
+++ b/pica-client/mainwindow.h
@@ -62,6 +62,12 @@ private:
 	void SetStatus(bool connected);
 	bool status_notifications_hidden;
 
+	// Mirrors SkyNet's online/offline state, kept in sync via the
+	// BecameSelfAware()/LostSelfAwareness() signals instead of querying
+	// skynet->isSelfAware() synchronously (that accessor was removed since
+	// SkyNet's state must only ever be read on the thread that owns it).
+	bool self_aware_cached;
+
 private slots:
 	void set_online();
 	void set_offline();
diff --git a/pica-client/skynet.cpp b/pica-client/skynet.cpp
index 0ce6882..8c6d404 100644
--- a/pica-client/skynet.cpp
+++ b/pica-client/skynet.cpp
@@ -17,6 +17,8 @@
 #include "skynet.h"
 #include "globals.h"
 #include <QMutex>
+#include <QMetaType>
+#include <QCoreApplication>
 //#include "dialogs/viewcertdialog.h"
 #include "contacts.h"
 #include "openssltool.h"
@@ -31,11 +33,13 @@
 #include <time.h>
 
 SkyNet::SkyNet()
-	: nodes(config_dbname), QObject(0), active_filetransfers(0)
+	: nodes(nullptr), QObject(0), active_filetransfers(0)
 {
 	self_aware = false;
 	call_waiting_c2c_connect = false;
 
+	qRegisterMetaType<Accounts::AccountRecord>("Accounts::AccountRecord");
+
 	PICA_client_callbacks cbs =
 	{
 		newmsg_cb,
@@ -79,10 +83,36 @@ SkyNet::SkyNet()
 	connect(this, SIGNAL(PeerCertificateReceived(QByteArray, QString, bool*)), this, SLOT(verify_peer_cert(QByteArray, QString, bool*)), Qt::DirectConnection);
 	connect(this, SIGNAL(MultiloginMessageReceived(quint64,QString,quint16)), this, SLOT(multilogin_event(quint64,QString,quint16)), Qt::QueuedConnection);
 
+	file_transfer_timer_id = 0;
+
+}
+
+SkyNet::~SkyNet()
+{
+	delete nodes;
+}
+
+// Name of the QSqlDatabase connection opened for this SkyNet instance's own
+// (network) thread - separate from the GUI thread's default connection,
+// since a QSqlDatabase connection can only be used by the thread that
+// created it.
+static const QString network_sql_connection_name = QStringLiteral("network_thread_connection");
+
+void SkyNet::init()
+{
+	QSqlDatabase netdb = QSqlDatabase::addDatabase("QSQLITE", network_sql_connection_name);
+	netdb.setDatabaseName(config_dbname);
+	netdb.open();
+
+	nodes = new Nodes(config_dbname, network_sql_connection_name);
+
 	event_loop_timer_id = startTimer(100);
 	c2c_reconnect_timer_id = startTimer(1000);
-	file_transfer_timer_id = 0;
+}
 
+void SkyNet::moveBackToMainThread()
+{
+	moveToThread(QCoreApplication::instance()->thread());
 }
 
 void SkyNet::active_filetransfers_up()
@@ -140,14 +170,14 @@ void SkyNet::nodelink_activated(PICA_c2n *c2n)
 		self_aware = true;
 		active_nodelink = c2n;
 
-		nodes.MakeClean();
+		nodes->MakeClean();
 
 		emit BecameSelfAware();
 
 		//load undelivered messages from history
 		if (msgqueues.isEmpty())
 		{
-			History h(config_dbname, Accounts::GetCurrentAccount().id);
+			History h(config_dbname, Accounts::GetCurrentAccount().id, network_sql_connection_name);
 
 			msgqueues = h.GetUndeliveredMessages();
 		}
@@ -234,7 +264,7 @@ void SkyNet::multilogin_event(quint64 timestamp, QString node_addr, quint16 node
 
 void SkyNet::node_status_changed(Nodes::NodeRecord nr, bool alive)
 {
-	nodes.UpdateStatus(nr, alive);
+	nodes->UpdateStatus(nr, alive);
 }
 
 void SkyNet::verify_peer_cert(QByteArray peer_id, QString cert_pem, bool *verified)
@@ -242,7 +272,7 @@ void SkyNet::verify_peer_cert(QByteArray peer_id, QString cert_pem, bool *verifi
 //    ViewCertDialog vcd;
 //    vcd.SetCert(cert_pem);
 //    vcd.exec();
-	Contacts cnt(config_dbname, Accounts::GetCurrentAccount().id);
+	Contacts cnt(config_dbname, Accounts::GetCurrentAccount().id, network_sql_connection_name);
 	QString stored_cert;
 
 
@@ -386,8 +416,7 @@ void SkyNet::timerEvent(QTimerEvent *e)
 	if (!self_aware && e->timerId() == nodelink_reconnect_timer_id)
 	{
 		killTimer(nodelink_reconnect_timer_id);
-		Accounts::AccountRecord acc = this->CurrentAccount();
-		Join(acc);
+		Join(skynet_account);
 	}
 }
 
@@ -417,15 +446,21 @@ bool SkyNet::open_account()
 	return true;
 }
 
-void SkyNet::Join(Accounts::AccountRecord &accrec)
+void SkyNet::Join(const Accounts::AccountRecord &accrec)
+{
+	QMetaObject::invokeMethod(this, "do_Join", Qt::AutoConnection,
+	                           Q_ARG(Accounts::AccountRecord, accrec));
+}
+
+void SkyNet::do_Join(const Accounts::AccountRecord &accrec)
 {
 	PICA_directc2c_config directc2c_cfg = PICA_DIRECTC2C_CFG_DISABLED;
 	int multilogin = PICA_MULTILOGIN_PROHIBIT;
-	Settings st(config_dbname);
+	Settings st(config_dbname, network_sql_connection_name);
 
 	skynet_account = accrec;
 
-	QList<Nodes::NodeRecord> noderecords = nodes.GetNodes();
+	QList<Nodes::NodeRecord> noderecords = nodes->GetNodes();
 
 	if (!acc)
 	{
@@ -522,6 +557,11 @@ void SkyNet::Join(Accounts::AccountRecord &accrec)
 
 void SkyNet::Exit()
 {
+	QMetaObject::invokeMethod(this, "do_Exit", Qt::AutoConnection);
+}
+
+void SkyNet::do_Exit()
+{
 	if (self_aware && active_nodelink)
 	{
 		PICA_close_c2n(active_nodelink);
@@ -542,12 +582,13 @@ void SkyNet::Exit()
 	emit LostSelfAwareness();
 }
 
-bool SkyNet::isSelfAware()
+void SkyNet::SendFile(QByteArray to, QString filepath)
 {
-	return self_aware;
+	QMetaObject::invokeMethod(this, "do_SendFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, to), Q_ARG(QString, filepath));
 }
 
-void SkyNet::SendFile(QByteArray to, QString filepath)
+void SkyNet::do_SendFile(QByteArray to, QString filepath)
 {
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
@@ -592,6 +633,12 @@ void SkyNet::SendFile(QByteArray to, QString filepath)
 
 void SkyNet::AcceptFile(QByteArray from, QString filepath)
 {
+	QMetaObject::invokeMethod(this, "do_AcceptFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, from), Q_ARG(QString, filepath));
+}
+
+void SkyNet::do_AcceptFile(QByteArray from, QString filepath)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -613,6 +660,12 @@ void SkyNet::AcceptFile(QByteArray from, QString filepath)
 
 void SkyNet::DenyFile(QByteArray from)
 {
+	QMetaObject::invokeMethod(this, "do_DenyFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, from));
+}
+
+void SkyNet::do_DenyFile(QByteArray from)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -632,6 +685,12 @@ void SkyNet::DenyFile(QByteArray from)
 
 void SkyNet::PauseFile(QByteArray peer_id, bool pause_sending)
 {
+	QMetaObject::invokeMethod(this, "do_PauseFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, peer_id), Q_ARG(bool, pause_sending));
+}
+
+void SkyNet::do_PauseFile(QByteArray peer_id, bool pause_sending)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -651,6 +710,12 @@ void SkyNet::PauseFile(QByteArray peer_id, bool pause_sending)
 
 void SkyNet::ResumeFile(QByteArray peer_id, bool resume_sending)
 {
+	QMetaObject::invokeMethod(this, "do_ResumeFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, peer_id), Q_ARG(bool, resume_sending));
+}
+
+void SkyNet::do_ResumeFile(QByteArray peer_id, bool resume_sending)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -669,6 +734,12 @@ void SkyNet::ResumeFile(QByteArray peer_id, bool resume_sending)
 
 void SkyNet::CancelFile(QByteArray peer_id, bool cancel_sending)
 {
+	QMetaObject::invokeMethod(this, "do_CancelFile", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, peer_id), Q_ARG(bool, cancel_sending));
+}
+
+void SkyNet::do_CancelFile(QByteArray peer_id, bool cancel_sending)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -687,6 +758,12 @@ void SkyNet::CancelFile(QByteArray peer_id, bool cancel_sending)
 
 void SkyNet::SendMessage(QByteArray to, QString msg)
 {
+	QMetaObject::invokeMethod(this, "do_SendMessage", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, to), Q_ARG(QString, msg));
+}
+
+void SkyNet::do_SendMessage(QByteArray to, QString msg)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -730,6 +807,12 @@ void SkyNet::SendMessage(QByteArray to, QString msg)
 
 void SkyNet::AcceptCall(QByteArray from)
 {
+	QMetaObject::invokeMethod(this, "do_AcceptCall", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, from));
+}
+
+void SkyNet::do_AcceptCall(QByteArray from)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -747,6 +830,12 @@ void SkyNet::AcceptCall(QByteArray from)
 
 void SkyNet::RejectCall(QByteArray from)
 {
+	QMetaObject::invokeMethod(this, "do_RejectCall", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, from));
+}
+
+void SkyNet::do_RejectCall(QByteArray from)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -764,6 +853,12 @@ void SkyNet::RejectCall(QByteArray from)
 
 void SkyNet::HangupCall(QByteArray with)
 {
+	QMetaObject::invokeMethod(this, "do_HangupCall", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, with));
+}
+
+void SkyNet::do_HangupCall(QByteArray with)
+{
 	int ret = PICA_OK;
 	struct PICA_c2c *iptr;
 
@@ -781,7 +876,13 @@ void SkyNet::HangupCall(QByteArray with)
 
 void SkyNet::StartCall(QByteArray to)
 {
-	if (!isSelfAware())
+	QMetaObject::invokeMethod(this, "do_StartCall", Qt::AutoConnection,
+	                           Q_ARG(QByteArray, to));
+}
+
+void SkyNet::do_StartCall(QByteArray to)
+{
+	if (!self_aware)
 	{
 		emit CallFailed(QString(tr("Pica Pica client is not online")));
 		return;
@@ -1127,7 +1228,7 @@ void SkyNet::c2c_closed_cb(const unsigned char *peer_id, int reason)
 void SkyNet::nodelist_cb(int type, void *addr_bin, const char *addr_str, unsigned int port)
 {
 	Nodes::NodeRecord nr = {addr_str, static_cast<quint16>(port)};
-	skynet->nodes.Add(nr);
+	skynet->nodes->Add(nr);
 }
 
 int SkyNet::peer_cert_verify_cb(const unsigned char *peer_id, const char *cert_pem, unsigned int nb)
diff --git a/pica-client/skynet.h b/pica-client/skynet.h
index c4846ab..64f0874 100644
--- a/pica-client/skynet.h
+++ b/pica-client/skynet.h
@@ -32,8 +32,8 @@ class SkyNet : public QObject
 	Q_OBJECT
 public:
 	SkyNet();
-	void Join(Accounts::AccountRecord &accrec);
-	bool isSelfAware();
+	~SkyNet();
+	void Join(const Accounts::AccountRecord &accrec);
 	void Exit();
 
 	void SendMessage(QByteArray to, QString msg);
@@ -49,11 +49,6 @@ public:
 	void RejectCall(QByteArray from);
 	void HangupCall(QByteArray with);
 
-	Accounts::AccountRecord CurrentAccount()
-	{
-		return skynet_account;
-	};
-
 signals:
 	void MessageReceived(QByteArray from, QString msg);
 	void UnableToDeliver(QByteArray to, QString msg);
@@ -89,7 +84,10 @@ signals:
 	void CallRejected(QByteArray by);
 	void CallHungup(QByteArray by);
 private:
-	Nodes nodes;
+	// Constructed in init(), once this SkyNet instance is running on its
+	// own network thread and that thread's dedicated QSqlDatabase
+	// connection has been opened - see init() in skynet.cpp.
+	Nodes *nodes;
 	QList<QPair<struct PICA_c2n *, Nodes::NodeRecord> > connecting_nodes;
 	QList<struct PICA_c2n *> connected_nodes_to_close;
 	bool self_aware;
@@ -225,6 +223,44 @@ private slots:
 	void node_status_changed(Nodes::NodeRecord nr, bool alive);
 	void verify_peer_cert(QByteArray peer_id, QString cert_pem, bool *verified);
 
+	// Connected to the network thread's QThread::started() signal. Runs
+	// once, on the network thread itself, right before that thread's
+	// event loop starts. Anything that needs to be tied to the network
+	// thread rather than whatever thread constructed this SkyNet
+	// instance - the event-loop timers and this object's own QSqlDatabase
+	// connection - is set up here instead of in the constructor.
+	void init();
+
+	// QObject::moveToThread() only succeeds when called from the thread
+	// the object is currently affiliated with - it cannot be "pulled" in
+	// from a different thread. So reclaiming this object for the main
+	// thread at shutdown has to happen via a call executed here, on the
+	// network thread itself (see main.cpp, invoked with
+	// Qt::BlockingQueuedConnection before the network thread is stopped),
+	// rather than by calling moveToThread() directly from main().
+	void moveBackToMainThread();
+
+	// Actual implementations behind the public action methods below.
+	// The public methods only forward here via QMetaObject::invokeMethod()
+	// with Qt::AutoConnection, so callers on any thread are automatically
+	// marshaled onto whichever thread this SkyNet instance lives on -
+	// direct call if already on that thread, queued otherwise.
+	void do_Join(const Accounts::AccountRecord &accrec);
+	void do_Exit();
+
+	void do_SendMessage(QByteArray to, QString msg);
+	void do_SendFile(QByteArray to, QString filepath);
+	void do_AcceptFile(QByteArray from, QString filepath);
+	void do_DenyFile(QByteArray from);
+	void do_PauseFile(QByteArray peer_id, bool pause_sending);
+	void do_ResumeFile(QByteArray peer_id, bool resume_sending);
+	void do_CancelFile(QByteArray peer_id, bool cancel_sending);
+
+	void do_StartCall(QByteArray to);
+	void do_AcceptCall(QByteArray from);
+	void do_RejectCall(QByteArray from);
+	void do_HangupCall(QByteArray with);
+
 };
 
 #endif // SKYNET_H