QTCP Socket Server and threads

[cafu1007]

Hi All,

I have an application that generates/obtain information information in the background, a pushed (signal) to a GUI and to a server class(inherits from QTCServer) which has the array of sockets(inherits from QTCPSocket)

The GUI live in the main thread.
The Server lives in working thread1 (everything is guard using mutex)
The Generation/acquiring information is in working thread2.

So the problem is i a deadlock.

Every single time there is information available, the server push it to all the clients connected to it.

The deadlock occur if the client send something to the server,
the socket(in the server side) is in the slotReadyRead reading the buffer.
and there some new information that needs to be send to the through the same sockets.
At this point the deadlock is present.

Is there some alternative?
is it really needed to have guards in socket class, thread-safe?
is it enough to guarantee that is re-entrant?
I assure that function in the socket is always called from the thread that creates it (thread1) using signal/slot.

Any help is appreciated.

CAFU

[wysota]

The Server lives in working thread1

What for?

Is there some alternative?

Yes, don't use threads if they don't give you any benefits.

[cafu1007]

Even if i dont use a thread for the server i still need a thread for the background, and when everything is guard with mutex the deadlock will be present again in similar situation.

[wysota]

Even if i dont use a thread for the server i still need a thread for the background

You probably don't need a thread for that too.

If you have a deadlock then it means you're not using mutexes properly. If you share some code, maybe we can come up with an alternative design.

[cafu1007]

OK here is some Code

code for CSocket and CSocketPrivate:

void CSocket::connectToHost(const QString &hostName, quint16 port)
{
	L_D(CSocket);
	QMutexLocker locker(&d->m_mMutex);
	d->connectToHost(hostName, port);
}
void CSocketPrivate::connectToHost(const QString &hostName, quint16 port)
{
	QTcpSocket::connectToHost(hostName, port);	
}
void CSocketPrivate::slotConnect(QString hostName, quint16 port)
{
	QMutexLocker locker(&m_mMutex);
	QTcpSocket::connectToHost(hostName, port);
	m_wcConnected.wakeAll();
}
qint64 CSocket::send(const CTCPFrame &frame)
{
	L_D(CSocket);
	QMutexLocker locker(&d->m_mMutex);
	return d->send(frame);
}
qint64 CSocketPrivate::send(const CFrame &frame)
{
	QByteArray data;
	data = frame.serialize();
	return send(data);;
}
qint64 CSocketPrivate::send(const QByteArray &message)
{
	qint64 Error = QTcpSocket::write(message);
	flush();
	return Error;
}
void CLLSocketPrivate::slotReadyRead()
{
	QMutexLocker locker(&m_mMutex);
	readTime->stop();
	readBuffer();
	readTime->start();
}

void CSocket::connectToHost(const QString &hostName, quint16 port)
{
	L_D(CSocket);
	QMutexLocker locker(&d->m_mMutex);
	d->connectToHost(hostName, port);
}
void CSocketPrivate::connectToHost(const QString &hostName, quint16 port)
{
	QTcpSocket::connectToHost(hostName, port);	
}
void CSocketPrivate::slotConnect(QString hostName, quint16 port)
{
	QMutexLocker locker(&m_mMutex);
	QTcpSocket::connectToHost(hostName, port);
	m_wcConnected.wakeAll();
}
qint64 CSocket::send(const CTCPFrame &frame)
{
	L_D(CSocket);
	QMutexLocker locker(&d->m_mMutex);
	return d->send(frame);
}
qint64 CSocketPrivate::send(const CFrame &frame)
{
	QByteArray data;
	data = frame.serialize();
	return send(data);;
}
qint64 CSocketPrivate::send(const QByteArray &message)
{
	qint64 Error = QTcpSocket::write(message);
	flush();
	return Error;
}
void CLLSocketPrivate::slotReadyRead()
{
	QMutexLocker locker(&m_mMutex);
	readTime->stop();
	readBuffer();
	readTime->start();
}

To copy to clipboard, switch view to plain text mode 

Code for Server and ServerPrivate:

CSvrNetControlPrivate::CSvrNetControlPrivate(QPointer<QObject> parent, qint32 port, bool bIpv6) :
		QTcpServer(parent)
{
	m_mMutex.lock();
	m_Thread.start();
	moveToThread(&m_Thread);
	m_mMutex.unlock();
}
void CSvrNetControlPrivate::incomingConnection(int socketDescriptor)
{
	QMutexLocker locker(&m_mMutex);
	newClient(socketDescriptor);
}
void CSvrNetControlPrivate::newClient(int socketDescriptor)
{
	if (QThread::currentThread() != &m_Thread)
	{
		emit
		signalNewClient(socketDescriptor);
		if (!m_wcNewClientDone.wait(&m_mMutex, 10000))
		{
			qWarning() << "CSvrNetControl::newClient m_wcNewClientDone no triggered";
			return;
		}
	}
	else
	{
		makeNewClient(socketDescriptor);
	}
}
void CSvrNetControlPrivate::slotNewClient(int socketDescriptor)
{
	QMutexLocker locker(&m_mMutex);
	makeNewClient(socketDescriptor);
	m_wcNewClientDone.wakeAll();
}
void CSvrNetControlPrivate::makeNewClient(int socketDescriptor)
{
	QPointer<CSocket> socket = new CSocket(this);
	if (!socket->setSocketDescriptor(socketDescriptor))
	{
		delete socket;
		socket = 0;
		return;
	}
	m_lSocket.append(socket);
	connect(m_lSocket.last(), SIGNAL(disconnected()), this, SLOT(slotConnectionClosed()));
	connect(m_lSocket.last(), SIGNAL(signalMessageReceived(CFrame)), this, SLOT(slotMessageReceived(CFrame)));
	L_Q(CSvrNetControl);
	emit q->signalNewClientConnected(m_lSocket.last()->peerAddress().toString());
}
void CSvrNetControl::broadcastMessage(const CFrame &frame)
{
	L_D(CSvrNetControl);	
	QMutexLocker locker(&d->m_mMutex);	
	return d->broadcastMessage(frame);
}
void CSvrNetControlPrivate::broadcastMessage(const CFrame &frame)
{
	for (qint32 i = 0; i < m_lSocket.size(); i++)
	{
		sendMessagePrivate(m_lSocket.at(i), frame);
	}	
}
void CSvrNetControlPrivate::sendMessagePrivate(QPointer<CSocket> client, const CFrame &frame)
{
	if ((QThread::currentThread() != &m_Thread))
	{
		emit signalSend(client, frame);
		if (!m_wcSended.wait(&m_mMutex, 10000))
		{
			qWarning() << "CSvrNetControl::sendMessagePrivate m_wcSended no triggered" << QThread::currentThreadId();
			m_eError = CErrorDescription(-1, "Not Performed");
		}
	}
	else
	{
		sendMessage(client, frame);
	}
}
void CSvrNetControlPrivate::slotSend(QPointer<CSocket> client, const CFrame &frame)
{
	QMutexLocker locker(&m_mMutex);
	sendMessage(client, frame);
	m_wcSended.wakeAll();
}
void CSvrNetControlPrivate::sendMessage(QPointer<CSocket> client, const CFrame &frame)
{
	if (client)
	{
		client->send(frame);
	}
}

CSvrNetControlPrivate::CSvrNetControlPrivate(QPointer<QObject> parent, qint32 port, bool bIpv6) :
		QTcpServer(parent)
{
	m_mMutex.lock();
	m_Thread.start();
	moveToThread(&m_Thread);
	m_mMutex.unlock();
}
void CSvrNetControlPrivate::incomingConnection(int socketDescriptor)
{
	QMutexLocker locker(&m_mMutex);
	newClient(socketDescriptor);
}
void CSvrNetControlPrivate::newClient(int socketDescriptor)
{
	if (QThread::currentThread() != &m_Thread)
	{
		emit
		signalNewClient(socketDescriptor);
		if (!m_wcNewClientDone.wait(&m_mMutex, 10000))
		{
			qWarning() << "CSvrNetControl::newClient m_wcNewClientDone no triggered";
			return;
		}
	}
	else
	{
		makeNewClient(socketDescriptor);
	}
}
void CSvrNetControlPrivate::slotNewClient(int socketDescriptor)
{
	QMutexLocker locker(&m_mMutex);
	makeNewClient(socketDescriptor);
	m_wcNewClientDone.wakeAll();
}
void CSvrNetControlPrivate::makeNewClient(int socketDescriptor)
{
	QPointer<CSocket> socket = new CSocket(this);
	if (!socket->setSocketDescriptor(socketDescriptor))
	{
		delete socket;
		socket = 0;
		return;
	}
	m_lSocket.append(socket);
	connect(m_lSocket.last(), SIGNAL(disconnected()), this, SLOT(slotConnectionClosed()));
	connect(m_lSocket.last(), SIGNAL(signalMessageReceived(CFrame)), this, SLOT(slotMessageReceived(CFrame)));
	L_Q(CSvrNetControl);
	emit q->signalNewClientConnected(m_lSocket.last()->peerAddress().toString());
}
void CSvrNetControl::broadcastMessage(const CFrame &frame)
{
	L_D(CSvrNetControl);	
	QMutexLocker locker(&d->m_mMutex);	
	return d->broadcastMessage(frame);
}
void CSvrNetControlPrivate::broadcastMessage(const CFrame &frame)
{
	for (qint32 i = 0; i < m_lSocket.size(); i++)
	{
		sendMessagePrivate(m_lSocket.at(i), frame);
	}	
}
void CSvrNetControlPrivate::sendMessagePrivate(QPointer<CSocket> client, const CFrame &frame)
{
	if ((QThread::currentThread() != &m_Thread))
	{
		emit signalSend(client, frame);
		if (!m_wcSended.wait(&m_mMutex, 10000))
		{
			qWarning() << "CSvrNetControl::sendMessagePrivate m_wcSended no triggered" << QThread::currentThreadId();
			m_eError = CErrorDescription(-1, "Not Performed");
		}
	}
	else
	{
		sendMessage(client, frame);
	}
}
void CSvrNetControlPrivate::slotSend(QPointer<CSocket> client, const CFrame &frame)
{
	QMutexLocker locker(&m_mMutex);
	sendMessage(client, frame);
	m_wcSended.wakeAll();
}
void CSvrNetControlPrivate::sendMessage(QPointer<CSocket> client, const CFrame &frame)
{
	if (client)
	{
		client->send(frame);
	}
}

To copy to clipboard, switch view to plain text mode 

[wysota]

You cannot access sockets from multiple threads. If you insist on using threads, each QObject doing something in the background (like QTcpSocket or QTcpServer) can only be accessed from a single thread. Mutexes will not help here, it's a matter of Qt's internal design. You have to post a request to an object living in the thread handling the socket and then handle that request in the proper thread.

However you really don't need any extra threads here. You can take the code above, strip it of all mutexes, run it in the gui thread and it will work just fine and faster than if you used threads.

[cafu1007]

You have to post a request to an object living in the thread handling the socket and then handle that request in the proper thread.

I assure this by signaling to the proper thread. this is working right...

However you really don't need any extra threads here. You can take the code above, strip it of all mutexes, run it in the gui thread and it will work just fine and faster than if you used threads.

So i am not getting any benefits from using threads here, but the amount of information being handle is very big. there is new values coming from serial ports (5) around 10 values/sec from each port that have to "deserialize", analyze and do specific actions.
The gui displays this information assign it to specific widget and has to send it to the clients, when there is many clients connected, will the gui have a latency?

[wysota]

I assure this by signaling to the proper thread. this is working right...

No, you don't assure anything. If you did, you wouldn't need mutexes. If you protect socket access by a mutex then it means that you expect more than one thread to access it.
The fact that you call a method from an object that is somehow associated with a thread, doesn't mean you call the method in the context of that thread.

So i am not getting any benefits from using threads here, but the amount of information being handle is very big. there is new values coming from serial ports (5) around 10 values/sec from each port that have to "deserialize", analyze and do specific actions.

Unless you have a really old machine, one thread is sufficient. If you have a really old machine, using more than one thread won't help if you have just a single CPU machine.

The gui displays this information assign it to specific widget and has to send it to the clients, when there is many clients connected, will the gui have a latency?

No, unless you have hundreds or thousands of high-traffic simultaneous connections (provided your network interface can cope with it). Currently you waste lot of time on locking and context switching without any benefits whatsoever.

[cafu1007]

No, you don't assure anything. If you did, you wouldn't need mutexes.

I use mutex if another thread try to access the CSocketPrivate class will emit a signal what will assure that the QTCPSocket functions are called in the thread that they have affinity...
maybe i dont need the usage of mutex for that, only with triggering the signal would be enough, or?

[wysota]

I use mutex if another thread try to access the CSocketPrivate class

You contradict yourself. Either only one thread accesses the data or not.

maybe i dont need the usage of mutex for that, only with triggering the signal would be enough, or?

You don't need threads. You're putting a lot of effort in trying to synchronize something that doesn't need synchronization. Just use signals and slots in one thread and it will work properly.

[cafu1007]

there is a public class(CSocket) and a private class(CSocketPrivate) if one want to send something have to go through the public class and if a this point the calling thread is not the one with the affinity a signal will be emitted,
i am not contradicting myself (I think ).

Well i have followed your the advice remove the threads, and mutex, is wroking good, Thanks.

[wysota]

there is a public class(CSocket) and a private class(CSocketPrivate) if one want to send something have to go through the public class and if a this point the calling thread is not the one with the affinity a signal will be emitted,
i am not contradicting myself (I think ).

Affinity of the signal doesn't matter. What matters is the context in which the socket is accessed.

I have one problem the client application is receiving very delayed what the server sends. Why? is it normal? how can i make the client to empty the buffer faster? the reading of the buffer is done using a timer with 50ms interval.

Hard for me to say without seeing the actual code. It might be that you're blocking execution of the thread with something (like a mutex or a blocking call to a device).

[cafu1007]

The problem of this solution without thread is if the main thread gets block because of re-sizing, moving the GUI or anything else that blocks the main thread, everything else will stop. And i need to know if a value from the one being reading from the serial port changes and its in a critical range to do an action according to this...

---

Added after 6 minutes:

Hard for me to say without seeing the actual code. It might be that you're blocking execution of the thread with something (like a mutex or a blocking call to a device).

There was stuff in the buffer but I was not making it empty properly.

[wysota]

The problem of this solution without thread is if the main thread gets block because of re-sizing, moving the GUI or anything else that blocks the main thread, everything else will stop.

The actions you mention do not block the thread.

And i need to know if a value from the one being reading from the serial port changes and its in a critical range to do an action according to this...

In that case using sockets is a bad idea at all. So is using a non-RT operating system. It can always happen that your application gets frozen by the operating system for a number of miliseconds. Threads will not help in any way here. Time critical conditions need real time operating systems. So it's either getting a real-time operating system or accepting that at times things can temporarily go out of control.

[cafu1007]

HI There, as i was recommended i am not using thread any where. Everything is running in a single thread (The main). but during debugging i see that at least 6 thread are created.

is the any Qt class that creates new thread silently?

[wysota]

is the any Qt class that creates new thread silently?

Yes, QFileSystemModel.

[ChrisW67]

Also QNetworkAccessManager for HTTP requests: http://blog.qt.digia.com/2011/04/29/...accessmanager/

[cafu1007]

HI There,

I was told that press down click event on the gui wil not stop the event loop.
I monitor what is send and received in the serial port. When i hold down the mouse over the title bar for moving the Windows, there is not receiving information (i would say not read) from the serial port, i used Qextserialport class for serial communication. is this a Qt problem, Qextserialport or simply a block on the event loop?

Thanks

[wysota]

This is somehow related to Windows display system. Qt has no influence on it since the title bar is not part of your application.

[cafu1007]

So how could i get around this. if i am not using threads. this is unwanted situation since everything should remain responding and communication should remain.