QTcpSocket::read() reads 0 bytes and it shouldn't: why?

Bart_Vandewoestyne

While debugging some mysterious network communication problem in our legacy codebase (Qt 4.8.7 on Windows), I was able to trim it down to the following:

First, we use waitForReadyRead() to check that there is new data available for reading from the QTcpSocket.
Next, once we get past waitForReadyRead(), there is new data available and just to make sure there are actually bytes there, I check with bytesAvailable() which tells me that there are 14 bytes available for reading.

In our legacy code base, the data is then read as follows:

char theByte = '\0';
while (theByte != someEndFlag)
{
    const auto bytesRead = read(&theByte, 1);
    ....
}

Note that the original authors decided to read one byte at a time. I believe this is not optimal, but for as far as I know, it should work. The mysterious thing happening here, is that bytesRead is 0, which is definitely not what I expect, because almost right before the read statement, bytesAvailable() tells me there are 14 bytes available! So why am I not reading the first of those 14 bytes then?

Oh, and by the way... this fails in a release build, but works in a debug build. For release builds where we show more log output, either to the console or to a file, it also works. So the problem only occurs in our 'fastest' setup, being a release build without output to the console or logfiles.

aha_1980

Hi @Bart_Vandewoestyne

I hope you use waitForReadyRead() in a separate thread, not in the main thread. That will most likely not work.
Have you set a timeout for waitForReadyRead()? From your description, it sounds like the whole code runs faster than you expect and is only slowed down by the debug prints - and therefore you don't see the bug when you enable the debug output.

jsulm

@Bart_Vandewoestyne said in QTcpSocket::read() reads 0 bytes and it shouldn't: why?:

while (theByte != someEndFlag)

I'm wondering what will happen if you read past these 14 bytes without getting someEndFlag? You will keep calling read(), right? And I think that's why you get bytesRead = 0 - because there is nothing more to read. This code really needs to be fixed:

while (theByte != someEndFlag)
{
    const auto bytesRead = read(&theByte, 1);
    if (bytesRead == 0)
       break;
    ....
}

aha_1980

@jsulm said in QTcpSocket::read() reads 0 bytes and it shouldn't: why?:

I'm wondering what will happen if you read past these 14 bytes without getting someEndFlag?

That is a very good point! Well spotted.

J.Hilk

I'm kinda confused, why, when reading a single char, the buffer is read in a while loop?
I mean the while loop either does not enter or breaks after the first cycle.

Is this for compiler optimization?

qint64 QIODevice::read(char *data, qint64 maxSize)
{
    Q_D(QIODevice);

#if defined QIODEVICE_DEBUG
    printf("%p QIODevice::read(%p, %lld), d->pos = %lld, d->buffer.size() = %lld\n",
           this, data, maxSize, d->pos, d->buffer.size());
#endif

    const bool sequential = d->isSequential();

    // Short-cut for getChar(), unless we need to keep the data in the buffer.
    if (maxSize == 1 && !(sequential && d->transactionStarted)) {
        int chint;
        while ((chint = d->buffer.getChar()) != -1) {
            if (!sequential)
                ++d->pos;

            char c = char(uchar(chint));
            if (c == '\r' && (d->openMode & Text))
                continue;
            *data = c;
#if defined QIODEVICE_DEBUG
            printf("%p \tread 0x%hhx (%c) returning 1 (shortcut)\n", this,
                   int(c), isprint(c) ? c : '?');
#endif
            if (d->buffer.isEmpty())
                readData(data, 0);
            return qint64(1);
        }
    }

    CHECK_MAXLEN(read, qint64(-1));
    CHECK_READABLE(read, qint64(-1));

    const qint64 readBytes = d->read(data, maxSize);

#if defined QIODEVICE_DEBUG
    printf("%p \treturning %lld, d->pos == %lld, d->buffer.size() == %lld\n", this,
           readBytes, d->pos, d->buffer.size());
    if (readBytes > 0)
        debugBinaryString(data - readBytes, readBytes);
#endif

    return readBytes;
}

Bart_Vandewoestyne

@jsulm said in QTcpSocket::read() reads 0 bytes and it shouldn't: why?:

I'm wondering what will happen if you read past these 14 bytes without getting someEndFlag? You will keep calling read(), right?

We first call waitForReadyRead again. The whole code looks more or less as follows (logging statements removed):

char theByte = '\0';
while (theByte != static_cast<char>(someEndFlag))
{
	const auto bytesRead = read(&theByte, 1);
	if (bytesRead > 0)
	{
		buffer.append(theByte);
		nbBytesRead++;
	}
	else  // error (-1) or no more data (0)
	{
		if (!waitForReadyRead(timeout))
		{
			buffer.clear();
			break;
		}
	}
}

In the meanwhile, we think we've found the root cause of the problem. I will explain that in another reply in this discussion.

Bart_Vandewoestyne

@J.Hilk said in QTcpSocket::read() reads 0 bytes and it shouldn't: why?:

I'm kinda confused, why, when reading a single char, the buffer is read in a while loop?
I mean the while loop either does not enter or breaks after the first cycle.

Is this for compiler optimization?

I'm afraid I do not completely understand your question. I've posted more code in another reply. Does that make things more clear for you? Note also that we think we've found the root cause of the problem. I will explain that in another reply in this discussion.

J.Hilk

@Bart_Vandewoestyne
hi, my comment was about the actually read function read(char *data, qint64 maxSize) , I posted the code frrom the QIODevice source code, your read ist most likly based on.
Can't thay for sure as I don't know the base class, but I would asume it to be QIODevice. As it is the case for nearly all read/write functions in Qt :-)

Bart_Vandewoestyne

We think we found the root cause of our problem. After some googling, I came across this Stack Overflow post: https://stackoverflow.com/questions/16653117/why-cant-my-code-read-the-number-of-bytes-available-on-a-qtcpsocket-ideas

The code I've posted, is part of a function mfReadDataFromDevice() which is a member function of a FooBarSocket class that is a subclass of QTcpSocket. To check the threads in which FooBarSocket::mfReadDataFromDevice() is called, and the thread in which the read() is called (= the thread where our socket object lives), I've added some logging statements right before the read() statement (before the while loop), where I print the value of QThread::currentThread() and thread().

And now here comes the clue: in some of our unit tests, these threads appeared to be the same, but in the failing unit test, these threads appeared to be different.

On top of that, we started to experience this problem after changing a signal/slot connection to a direct function call. The signal was only connected to a single slot, and we therefore thought we could simply replace the signal/slot connection by a direct function call. But there, we overlooked that the thread in which the socket lives could be different from the thread where FooBarSocket::mfReadDataFromDevice() is called. Once we reverted that change, the unit test no longer failed.

So our main conclusion would be: always make sure that the socket your are reading from belongs to the same thread on which you want to do the reading.

My remaining questions are:

Do you guys think that our conclusion is right?
This seems to be something to pay attention to. Is this something that is documented for QTcpSockets? Or is it something to pay attention to for even more general things, not only when working with QTcpSockets? I would be happy to have some more URLs/references to documentation/blogposts/other websites that talk about these pitfalls.
Given what we experienced/osberved, are there differences in behavior between Qt 4.8.7 (what we currently use) and Qt 5.X?

Kind regards,
Bart

kshegunov

@J.Hilk
I'd bet that's because of the text-mode translation for windows, see the comparison with '\r'

@Bart_Vandewoestyne said in QTcpSocket::read() reads 0 bytes and it shouldn't: why?:

Do you guys think that our conclusion is right?

If your investigation in the threading is correct, which I have no reason to doubt, then yes, this does sound right.

This seems to be something to pay attention to. Is this something that is documented for QTcpSockets? Or is it something to pay attention to for even more general things, not only when working with QTcpSockets? I would be happy to have some more URLs/references to documentation/blogposts/other websites that talk about these pitfalls.

Well, accessing objects, sockets or otherwise, from different threads has to be protected (serialized in some way). With signal slot connections Qt does that through the event loop, if you do it manually you have to use a mutex or a wait condition. That is if the class is not explicitly documented as thread safe (at the top of the class page)

Given what we experienced/osberved, are there differences in behavior between Qt 4.8.7 (what we currently use) and Qt 5.X?

If the reason you deduced is correct, the behaviour you get is undefined, as this is a race condition. So whether or not there's a difference will vary (or not) between Qt version, compiler and OS.

Bart_Vandewoestyne

Thanks for all the help an suggestions here. I always like it when I learn from mistakes, either my own or mistakes from others :-) Considering this closed now.