QTcpSocket can't read all bytes

[Qiieha]

Hi

a server sends 162076 bytes to the client. The client reads the QTcpSocket and gets only 16384 bytes back and the program crashes.

What's the best way to read all data?


QByteArray byteblock = tcpsocket->read();
QDataStream stream(&byteblock, QIODevice::ReadOnly);
QString str;
stream >> str;
qDebug() << "received:" << str;

Debug output is: received ""

thank u!

[wysota]

No, this is definitely wrong. Why are you using QDataStream?

[Qiieha]

thank u for your reply!
I saw this solution in some examples...

what should I use instead?
can u suggest something

[wysota]

Instead don't use QDataStream if you don't know what it does :)

[Qiieha]

what should I use?

[wysota]

You can use QTcpSocket, that you are already using. But basically everything depends on what the server is sending.

[Qiieha]

the server writes a QByteArray to the socket:


tcpsocket->write(bytearray);


and the bytearray has a size of 162076

[wysota]

Then read a byte array. But remember TCP is a stream protocol, don't expect to receive all data in one piece (that's the main reason why your datastream code was incorrect).

[Qiieha]

I try it like this:


void Interface::readyReadSlot()
{
qDebug() << "readyReadSlot()";
QByteArray byteblock;
while (!tcpsocket->atEnd()) {
QByteArray data = tcpsocket->read(100);
byteblock += data;
}
QDataStream stream(&byteblock, QIODevice::ReadOnly);
QString string;
stream >> string;
qDebug() << "received:" << in_string;
}

if the server sends a too long bytearray the Debug-Output is received " ", otherwise it's correct.
What's wrong?

[wysota]

I think you really need to learn how TCP works. There is no concept of a socket being "at end". Your approach is simply incorrect. When data arrives, append it to a buffer and when you have all the data you expect then make use of it. Of course you need to know how much data to expect. Such information needs to be encoded in the data stream (for instance you can use the newline character as a request delimiter or you can prepend the size of the record you are sending to the stream).

[Qiieha]

the datastream consists of XML, so I could use the </end> tag as information, couldn't I?

[wysota]

No, you couldn't. You need to trace the whole document tree and see when the document is complete. QXmlStreamReader might help you with it.

[Qiieha]

If I know the tagname of the last tag it should work:


void Interface::readyReadSlot()
{
qDebug() << "readyReadSlot()";
buffer.open(QIODevice::WriteOnly);
QByteArray parcel = tcpsocket->readAll();
QDataStream stream(&parcel, QIODevice::ReadOnly);
QString string;
stream>> string;
buffer.write(parcel);
buffer.close();
qDebug() << "parcelsize:" << QString::number(parcel.size());
qDebug() << string;
if(!string.contains(QRegExp("</EndTag>\n"))){
qDebug() << "wait";
tcpsocket->waitForReadyRead();
}
else
emit dataWritten(block);
}

if the server sends exactly one parcel this works fine, but if the server sends more than one parcels, the strings are empty and the buffer writes no data to the bytearray. So maybe the way of sending the data from server is incorrect:


QDataStream out(&bytearray, QIODevice::WriteOnly);
QString string= message;
out<< string;
tcpsocket->write(bytearray);

while(!tcpsocket->waitForBytesWritten());


:( Why it didn't work? thank u for your help

[wysota]

If I know the tagname of the last tag it should work
No. At least not in general case.




void Interface::readyReadSlot()
{
qDebug() << "readyReadSlot()";
buffer.open(QIODevice::WriteOnly);
QByteArray parcel = tcpsocket->readAll();
QDataStream stream(&parcel, QIODevice::ReadOnly);
QString string;
stream>> string;
buffer.write(parcel);
buffer.close();
qDebug() << "parcelsize:" << QString::number(parcel.size());
qDebug() << string;
if(!string.contains(QRegExp("</EndTag>\n"))){
qDebug() << "wait";
tcpsocket->waitForReadyRead();
}
else
emit dataWritten(block);
}


No, it won't work, because you are using QDataStream again. And the logic is flawed, including the call to waitForReadyRead().

I already gave you the exact recipe:
1. append all pending data to a buffer
2. inspect the buffer starting from the beginning until you see the "end of record" mark (or detect the end of record based on any other means available)
3. if there is not end of record mark, just return from the function and continue when you're called next time
4. if there is the end of record mark, remove the record from the buffer and process it leaving the rest of pending data intact.

Note: the whole point of having a buffer is for it to persist across calls to your readyRead handler.

[Qiieha]

ok, i'll try this


void Interface::readyReadSlot()
{
qDebug() << "readyReadSlot()";
buffer.open(QIODevice::WriteOnly);
QByteArray parcel = tcpsocket->readAll();
buffer.write(parcel);
buffer.close();
qDebug() << "parcelsize:" << QString::number(parcel.size());
if(!detectEndTag())
return();
else{
processData();
//remove data from buffer;
//QEventLoop::quit();
}
}

and the way of sending should work?

[wysota]

No, it's still wrong, you are discarding previous content of the buffer.
This is correct (by the way, there are numerous threads in this forum where I explain this particular situation, how come you didn't encounter any of the threads while using our search?):

QByteArray buffer; // global or member variable
void Cls::onSocketReadyRead() {
buffer.append(socket->readAll());
tryProcessData();
}

void Cls::tryProcessData() {
forever {
// for the sake of the example I'm assuming my record is always 24 bytes long
if(buffer.size()<24) return;
QByteArray record = buffer.left(24);
processRecord(record);
}
}

The above code has one small flaw introduced for purpose so that you need to understand completely what the code does before actually using it.

[Qiieha]

ok thank u!!
the return-statement after the processRecord(record) method is missing, isn't it?

[wysota]

No, that's not a problem. The forever loop wouldn't make sense then. Think why the loop is there.

[Qiieha]

the loop makes sure that no different method is executed until the record is not complete. A QEventLoop would be a different solution.

[wysota]

the loop makes sure that no different method is executed until the record is not complete.
No, that's not the reason. And no other method would be executed anyway since there is only one thread here. There is one line of code missing from my snippet. Analyze what the whole method does line by line and you'll quickly discover what's missing. But you need to understand why the forever loop is there in the first place, it's the heart of this code.

[Qiieha]

I don't know. Do you mean:


buffer.clear();

but it works...

[wysota]

No, then bufferring wouldn't make any sense. What is the purpose of the 'forever' loop?

[Qiieha]

That's logic. The buffer has to b cleared after processing record.
forever loop is a infinite loop.

[wysota]

No, clearing the buffer after processing a record is wrong. What if you get more than one message in one chunk (one call to onReadyRead())? Clearing the buffer will discard everything, including the data you haven't processed.

[Qiieha]

can you explain the thing, please.
I want to understand it.

[wysota]

From the beginning: the fact that sender sends "AAA" and then it sends "BBB" doesn't mean that the receiver will receive "AAA" and then "BBB". He can receive "AAABBB" or "AA" and "ABBB" or "AAABB" and "B" or even "A", "A", "A", "B", "B", "B". If your record was three characters long and you received "AAABB" and "B" then after processing "AAABB" and extracting "AAA" from it, you would discard "BB" and when you are called again with the last "B", you would fall out of sync because of the missing two characters. You have two autonomous systems talking to each other and TCP does not know anything such as "record", it just transmits bytes as they flow in. If this explanation is not enough for you then I'm sorry but you'll have to read some book or paper on TCP.

[Qiieha]

Ok I understand how TCP works, but in my case the client sends a request to the server. After that the client waits for response from server.
All the bytes from server are written into the bytearray until the client notices that the response is complete.
If the response is completed the client process the response and serializes the data.

So I can be sure that two responses can't be mixed.

[yeye_olive]

@Qiieha
Even if I designed a client and a server so that they sent 1 message to each other in turns, I would not write the networking code with that assumption in mind. That would work but would be bad practice and not very robust.

In any case, you need a way for the receiver to know the length of the message it reads, by using a delimiter sequence, prepending the message with its length, using a fixed length, you name it.