QTcpSocket readyRead strange behavior

[high_flyer]

Your code in the first post does NOT wait for the expected size:

qint64 nBytes = pSocket->bytesAvailable(); //you check bytes available
 
	qDebug()<<"onSocketReadyRead bytes="<<nBytes;
 
	if (nBytes < 2) // it could be you get 0 or 1 bytes, so you get out here. But lets say you got 3, so we go on.
		return;
	qDebug()<<"Bytes="<<nBytes;
	pSocket->peek((char*)&sSize, 2);//why do you use peek and not bytesAvailable? also, why do you peel only 2 bytes ahead? it will alway be at most 2 bytes this the following if will always get out.
 
	qDebug()<<"Packet size="<<sSize;
 
if (sSize != 19998) //so we didn't read 19998, (sSize will be at most 2 because of the peek) which is the size you are waiting for, so we get in to the if.
	{
		QMessageBox::critical(NULL, tr("error"), tr("This is it"));
		pSocket->disconnectFromHost();
		return; //And what do you do? you get out!!
	}
 
	if (nBytes < 2 + sSize)
	{
		qDebug()<<"  not enough data in stream";
		return;
	}

qint64 nBytes = pSocket->bytesAvailable(); //you check bytes available

	qDebug()<<"onSocketReadyRead bytes="<<nBytes;
	
	if (nBytes < 2) // it could be you get 0 or 1 bytes, so you get out here. But lets say you got 3, so we go on.
		return;
	qDebug()<<"Bytes="<<nBytes;
	pSocket->peek((char*)&sSize, 2);//why do you use peek and not bytesAvailable? also, why do you peel only 2 bytes ahead? it will alway be at most 2 bytes this the following if will always get out.

	qDebug()<<"Packet size="<<sSize;

if (sSize != 19998) //so we didn't read 19998, (sSize will be at most 2 because of the peek) which is the size you are waiting for, so we get in to the if.
	{
		QMessageBox::critical(NULL, tr("error"), tr("This is it"));
		pSocket->disconnectFromHost();
		return; //And what do you do? you get out!!
	}

	if (nBytes < 2 + sSize)
	{
		qDebug()<<"  not enough data in stream";
		return;
	}

To copy to clipboard, switch view to plain text mode 

EDIT:

if (sSize != 19998)

if (sSize != 19998)

To copy to clipboard, switch view to plain text mode 

is a tottaly a problem.
Read the peek documentation and be sure to understand it.
sSize should be a buffer, as long as the the data you want to read.
It does not store the size read size, but the read data!
If at all, you probably then use it like this:

int iSize = peel(buff,maySize);
 
if(iSize != iExpectedSize){...}

int iSize = peel(buff,maySize);

if(iSize != iExpectedSize){...}

To copy to clipboard, switch view to plain text mode 

[naroin]

the buffer i want to stream contains:
2 bytes, containing the size of the buffer
rest of the buffer.
in the final program, the size may differ from one packet to another, so i have to put it at the beginning of the send data.
For my tests here, i put it at 19998.

the goal is to read
* the size (in the 2 first bytes)
* then the buffer, corresponding to size just read
then redo the same.

here's my code with comments:

void Client::onSocketReadyRead()
{
	short sSize;
	char* data;
	unsigned long lPacketIndex;
	qint64 nBytes = pSocket->bytesAvailable();
 
	qDebug()<<"onSocketReadyRead bytes="<<nBytes;
 
	if (nBytes < 2) //we haven't read enough data to get the buffer size, return
		return;
	qDebug()<<"Bytes="<<nBytes;
	pSocket->peek((char*)&sSize, 2); //we read 2 bytes, we get the size of the buffer INSIDE these 2 bytes
//here i use peek to get the 2 bytes containing the size, without removing it from the buffer, because i may have not enough data, and i'll have to redo it.
 
	qDebug()<<"Packet size="<<sSize;
/* this IF is only to get easily the error . It's DEBUG code.
    as i KNOW the size must be 19998, if it's not, there was an error.
*/
	if (sSize != 19998) 
	{
//i put a breakpoint here, the code stops.
		QMessageBox::critical(NULL, tr("error"), tr("This is it"));
		pSocket->disconnectFromHost();
		return;
	}
 
	if (nBytes < 2 + sSize) //here we wait to have enough data to read sSize bytes
	{
		qDebug()<<"  not enough data in stream";
		return;
	}
 
	data = new char[sSize];
 
	pSocket->read((char*)&sSize, 2);
	pSocket->read(data, sSize);
 
	memcpy((char*)&lPacketIndex, data, 4);
 
	qDebug()<<"  data read, packet index="<<lPacketIndex;
 
	delete data;
}

void Client::onSocketReadyRead()
{
	short sSize;
	char* data;
	unsigned long lPacketIndex;
	qint64 nBytes = pSocket->bytesAvailable();

	qDebug()<<"onSocketReadyRead bytes="<<nBytes;
	
	if (nBytes < 2) //we haven't read enough data to get the buffer size, return
		return;
	qDebug()<<"Bytes="<<nBytes;
	pSocket->peek((char*)&sSize, 2); //we read 2 bytes, we get the size of the buffer INSIDE these 2 bytes
//here i use peek to get the 2 bytes containing the size, without removing it from the buffer, because i may have not enough data, and i'll have to redo it.

	qDebug()<<"Packet size="<<sSize;
/* this IF is only to get easily the error . It's DEBUG code.
    as i KNOW the size must be 19998, if it's not, there was an error.
*/
	if (sSize != 19998) 
	{
//i put a breakpoint here, the code stops.
		QMessageBox::critical(NULL, tr("error"), tr("This is it"));
		pSocket->disconnectFromHost();
		return;
	}

	if (nBytes < 2 + sSize) //here we wait to have enough data to read sSize bytes
	{
		qDebug()<<"  not enough data in stream";
		return;
	}

	data = new char[sSize];

	pSocket->read((char*)&sSize, 2);
	pSocket->read(data, sSize);

	memcpy((char*)&lPacketIndex, data, 4);

	qDebug()<<"  data read, packet index="<<lPacketIndex;

	delete data;
}

To copy to clipboard, switch view to plain text mode 

is it clearer?

[high_flyer]

wysota gave you the solution, why don't you use it?

[naroin]

I have answrered : i've exactly the same problem with wysota's solution.

As i already said, it works well for a certain time, then it randomly fail. In addition, it does not fail in a "good" network with no TCP retransmission, and often fails in a "bad" network, like low wifi.

well, i'll use directly windows sockets.

[wysota]

Can we see the code for the sender? Also are the sender and the receiver running the same operating system and the same CPU type?

[naroin]

yes, they are both on windows,

here's the server side code :

#define DATA_SIZE 20000
 
Server::Server(QObject *parent)	: QThread(parent)
{
 
}
 
Server::~Server()
{
 
}
 
void Server::run()
{
	pServer = new QTcpServer();
	pMapperReadyRead = new QSignalMapper();
	pMapperDisconnected = new QSignalMapper();
	pMapperError = new QSignalMapper();
 
	connect(pServer, SIGNAL(newConnection()), this, SLOT(onNewConnection()), Qt::DirectConnection);
	connect(pMapperReadyRead, SIGNAL(mapped(QObject*)), this, SLOT(onSocketReadyRead(QObject*)), Qt::DirectConnection);
	connect(pMapperDisconnected, SIGNAL(mapped(QObject*)), this, SLOT(onSocketDisconnected(QObject*)), Qt::DirectConnection);
 
	pServer->listen(QHostAddress::Any, 1234);
 
	//timer d'envoi
	lPacketIndex = 0;
	QTimer* pTimer = new QTimer(this);
	connect(pTimer, SIGNAL(timeout()), this, SLOT(onTimeout()));
 
	pTimer->setSingleShot(false);
	pTimer->start(100);	
 
 
 
	exec();
 
	pServer->close();
	delete pServer;
 
	delete pMapperReadyRead;
	delete pMapperDisconnected;
	delete pMapperError;
}
 
void Server::onNewConnection()
{
	QTcpSocket* pSocket = pServer->nextPendingConnection();
	if (pSocket == NULL)
		return;
 
	qDebug()<<"new connection";
 
	vecClient.push_back(pSocket);
 
	connect(pSocket, SIGNAL(readyRead()), pMapperReadyRead, SLOT(map()), Qt::DirectConnection);
	pMapperReadyRead->setMapping(pSocket, pSocket);
	connect(pSocket, SIGNAL(disconnected()), pMapperDisconnected, SLOT(map()), Qt::DirectConnection);
	pMapperDisconnected->setMapping(pSocket, pSocket);
}
 
void Server::onSocketReadyRead( QObject* pObject )
{
 
}
 
void Server::onSocketDisconnected( QObject* pObject )
{
	int i;
	QTcpSocket* pSocket = (QTcpSocket*)pObject;
 
	qDebug()<<"Socket disconnected";
 
	for (i=0;i<vecClient.size();++i)
	{
		if (pSocket = vecClient[i])
		{
			vecClient.erase(vecClient.begin() + i);
			break;
		}
	}
 
	pSocket->deleteLater();
}
 
void Server::onSocketError( QObject* pObject, QAbstractSocket::SocketError error )
{
	QTcpSocket* pSocket = (QTcpSocket*)pObject;
 
	qDebug()<<"Socket error : "<<pSocket->errorString();
}
 
void Server::onTimeout()
{
	int i;
	char data[DATA_SIZE];
	short sSize = DATA_SIZE - 2;
 
 
	memset(data, 0, DATA_SIZE);
	memcpy(data, &sSize, 2);
	memcpy(data+2, &lPacketIndex, 4);
 
	++lPacketIndex;
 
	for (i=0;i<vecClient.size();++i)
	{	
		vecClient[i]->write(data, DATA_SIZE);
 
	}
}

#define DATA_SIZE 20000

Server::Server(QObject *parent)	: QThread(parent)
{

}

Server::~Server()
{

}

void Server::run()
{
	pServer = new QTcpServer();
	pMapperReadyRead = new QSignalMapper();
	pMapperDisconnected = new QSignalMapper();
	pMapperError = new QSignalMapper();

	connect(pServer, SIGNAL(newConnection()), this, SLOT(onNewConnection()), Qt::DirectConnection);
	connect(pMapperReadyRead, SIGNAL(mapped(QObject*)), this, SLOT(onSocketReadyRead(QObject*)), Qt::DirectConnection);
	connect(pMapperDisconnected, SIGNAL(mapped(QObject*)), this, SLOT(onSocketDisconnected(QObject*)), Qt::DirectConnection);

	pServer->listen(QHostAddress::Any, 1234);

	//timer d'envoi
	lPacketIndex = 0;
	QTimer* pTimer = new QTimer(this);
	connect(pTimer, SIGNAL(timeout()), this, SLOT(onTimeout()));

	pTimer->setSingleShot(false);
	pTimer->start(100);	



	exec();

	pServer->close();
	delete pServer;

	delete pMapperReadyRead;
	delete pMapperDisconnected;
	delete pMapperError;
}

void Server::onNewConnection()
{
	QTcpSocket* pSocket = pServer->nextPendingConnection();
	if (pSocket == NULL)
		return;

	qDebug()<<"new connection";

	vecClient.push_back(pSocket);

	connect(pSocket, SIGNAL(readyRead()), pMapperReadyRead, SLOT(map()), Qt::DirectConnection);
	pMapperReadyRead->setMapping(pSocket, pSocket);
	connect(pSocket, SIGNAL(disconnected()), pMapperDisconnected, SLOT(map()), Qt::DirectConnection);
	pMapperDisconnected->setMapping(pSocket, pSocket);
}

void Server::onSocketReadyRead( QObject* pObject )
{

}

void Server::onSocketDisconnected( QObject* pObject )
{
	int i;
	QTcpSocket* pSocket = (QTcpSocket*)pObject;

	qDebug()<<"Socket disconnected";

	for (i=0;i<vecClient.size();++i)
	{
		if (pSocket = vecClient[i])
		{
			vecClient.erase(vecClient.begin() + i);
			break;
		}
	}

	pSocket->deleteLater();
}

void Server::onSocketError( QObject* pObject, QAbstractSocket::SocketError error )
{
	QTcpSocket* pSocket = (QTcpSocket*)pObject;

	qDebug()<<"Socket error : "<<pSocket->errorString();
}

void Server::onTimeout()
{
	int i;
	char data[DATA_SIZE];
	short sSize = DATA_SIZE - 2;
	

	memset(data, 0, DATA_SIZE);
	memcpy(data, &sSize, 2);
	memcpy(data+2, &lPacketIndex, 4);

	++lPacketIndex;

	for (i=0;i<vecClient.size();++i)
	{	
		vecClient[i]->write(data, DATA_SIZE);
			
	}
}

To copy to clipboard, switch view to plain text mode 

i can also send you VC++ projects, if you need them

[wysota]

This code is not compliant with what I have shown you. You have not implemented what I suggested so do that first and then we can continue.

[naroin]

yeah, that's my server.

the client connects to it, then the server regulary sends data (onTimeout)
In my project, it's the server that pushes bytes in the socket.

as you can see line 108 :

vecClient[i]->write(data, DATA_SIZE);

vecClient[i]->write(data, DATA_SIZE);

To copy to clipboard, switch view to plain text mode 

[wysota]

You are sending raw bytes and interpreting them on the other end as integers. Don't do that. I told you to encode the size explicitly. If your sender is a 32 bit OS and your receiver a 64 bit OS (or the other way round) your code will fail immediately.

[naroin]

you're totally right, but for this example, that's not the point : the code does not fail immediately.
As i am on 2 32bits windows, it works.

(does the size of "short" change between 32 and 64bit OS? i though it could be a problem only if the endianness changed)

[wysota]

It is the point because it is this exact variable that is giving you an incorrect result. So first make sure the problem is not there (and not assume it is not there) and then continue elsewhere.

This is sender side compatible with my previous suggestion.

QByteArray data; // external byte array containing your data to be sent
 
quint16 s = data.size();
QByteArray ba(2, 0);
ba[0] = (s >> 8) & 0xFF;
ba[1] = s & 0xFF;
ba.append(data);
for (i=0;i<vecClient.size();++i)
{	
	vecClient[i]->write(ba);
}

QByteArray data; // external byte array containing your data to be sent

quint16 s = data.size();
QByteArray ba(2, 0);
ba[0] = (s >> 8) & 0xFF;
ba[1] = s & 0xFF;
ba.append(data);
for (i=0;i<vecClient.size();++i)
{	
	vecClient[i]->write(ba);
}

To copy to clipboard, switch view to plain text mode 

[naroin]

i've got the same problem.

receiver side code :

void Client::onSocketReadyRead()
{
	bufferSocket += pSocket->readAll();
 
	while (bufferSocket.size() >= 2)
	{
		quint16 size = (bufferSocket.at(0) << 8) + bufferSocket.at(1); // 16bit unsigned value
 
		qDebug()<<"size="<<size;
 
		if (size != 19998)
		{
 
			QMessageBox::critical(NULL, tr("error"), tr("This is it"));
			pSocket->disconnectFromHost();
			return;
		}
 
		if(bufferSocket.size() <= 2+size) return;
 
		qDebug()<<"data read";
 
		QByteArray data = bufferSocket.mid(2, size);
 
		bufferSocket.remove(0,2+size);		
	}

void Client::onSocketReadyRead()
{
	bufferSocket += pSocket->readAll();

	while (bufferSocket.size() >= 2)
	{
		quint16 size = (bufferSocket.at(0) << 8) + bufferSocket.at(1); // 16bit unsigned value
		
		qDebug()<<"size="<<size;

		if (size != 19998)
		{

			QMessageBox::critical(NULL, tr("error"), tr("This is it"));
			pSocket->disconnectFromHost();
			return;
		}

		if(bufferSocket.size() <= 2+size) return;

		qDebug()<<"data read";

		QByteArray data = bufferSocket.mid(2, size);

		bufferSocket.remove(0,2+size);		
	}

To copy to clipboard, switch view to plain text mode 

sender side code:

void Server::onTimeout()
{
	QByteArray data; 
	int i;
 
	data.fill(1, 19998);
 
	quint16 s = data.size();
	QByteArray ba(2, 0);
	ba[0] = (s >> 8) & 0xFF;
	ba[1] = s & 0xFF;
	ba.append(data);
	for (i=0;i<vecClient.size();++i)
	{	
		vecClient[i]->write(ba);
	}
}

void Server::onTimeout()
{
	QByteArray data; 
	int i;
 
	data.fill(1, 19998);

	quint16 s = data.size();
	QByteArray ba(2, 0);
	ba[0] = (s >> 8) & 0xFF;
	ba[1] = s & 0xFF;
	ba.append(data);
	for (i=0;i<vecClient.size();++i)
	{	
		vecClient[i]->write(ba);
	}
}

To copy to clipboard, switch view to plain text mode 

[tbscope]

Note that the slot you call from the thread is not performed inside the thread.

[marcvanriet]

Hi,
You could look at the fortunecookie server and client examples.

In that example the length of the "packet" is put upfront. On the receiving side, the length is read, and they keep on receiving until all the bytes are received. This is what you want.

It is best to see a TCP connection as a 'stream' of bytes that are coming in. You have to find the start and end of your 'message' yourself.

Best regards,
Marc

[wysota]

In that example the length of the "packet" is put upfront. On the receiving side, the length is read, and they keep on receiving until all the bytes are received. This is what you want.

That's exactly what he has, you know

[marcvanriet]

Oops... didn't go to the very last page of this thread. Well, better safe then sorry...