PLEASE HELP... Weird problem with heap corruption when using QTcpSocket.readAll()

[primem0ver]

NOTE/UPDATE: I have figured out part of the problem and comment on it in the first reply.

I am getting a "User Breakpoint" in _CrtIsValidHeapPointer(). This function is located in dbgheap.c. I have no idea what could be causing this... According to the call stack it is occurring while an object is being deleted. But since I am not trying to delete anything in the code where it occurs I am not certain where to even look for a problem.

Some background. I am trying to create a multi-threaded server where each connection made is handled in a separate thread. I have used two different examples of threaded servers found on the internet as a model for my code and both have the same result.

Here is last code within my project that is executed before I get a "User Breakpoint" in _CrtIsValidHeapPointer() The address of the object that is being "deleted' seems like a legitimate address but I don't even know where to check since my code isn't directly responsible for the error. (IF it is responsible, it is an indirect relationship).

@
void LoginConnection::readyRead()
{
QByteArray data = mSocket->readAll(); // THIS LINE results in the error
emit loginReceived(data);
}
@

FYI... in case it helps I have included relevant class code.
Here is the listener (server) method that creates the socket thread:
@
void LoginServer::incomingConnection(qintptr socketDescriptor)
{
ILiveServerLogHost* logHost = mEngine->getLiveServerHost();
NetEventInfo* netEvent = new NetEventInfo(QDateTime::currentDateTime(), NULL, NULL, NetMessageTypes::login, true);

/// XXX should write to event log
//emit incomingSignal(netEvent);

LoginConnection* connection = new LoginConnection(socketDescriptor, netEvent, mEngine);
connect(connection, SIGNAL(loginReceived(const QByteArray& data)), this, SLOT(processLogin(const QByteArray&)));
connect(connection, SIGNAL(finished()), connection, SLOT(deleteLater()));
connection->start();
}
@

Here is the class inherited from QThread where it occurs...

@
class LoginConnection : public QThread
{
Q_OBJECT
public:
explicit LoginConnection(qintptr ID, NetEventInfo* requestEvent, IServerEngine* engine, QObject* parent = 0);
void run();
void sendLoginResponse(QByteArray* response);

signals:
void socketError(QTcpSocket::SocketError socketError);
void loginReceived(const QByteArray& data);
void loginInitiated(NetEventInfo* requestEvent);
void loginFinalized(NetEventInfo* sendEvent);

public slots:
void readyRead();
void disconnected();

private:
qintptr mSocketID;
QTcpSocket* mSocket;
NetEventInfo* mRequestEvent;
QByteArray* mResponse;
bool mQuit;
//IServerEngine* mEngine;

};
@

Here is the code that sets up the signal slot connections, including the one that leads to the error.

@
LoginConnection::LoginConnection(qintptr ID, NetEventInfo* requestEvent, IServerEngine* engine, QObject* parent) :
QThread(parent),
mSocketID(ID),
mRequestEvent(requestEvent),
mQuit(false),
mResponse(NULL),
mSocket(NULL)
{
connect(this, SIGNAL(loginInitiated(NetEventInfo* newRequestEvent)), engine, SLOT(loginReceived(NetEventInfo* newRequestEvent)), Qt::QueuedConnection);
connect(this, SIGNAL(loginFinalized(NetEventInfo* sendEvent)), engine, SLOT(loginSent(NetEventInfo* sendEvent)), Qt::QueuedConnection);
}

void LoginConnection::run()
{
mSocket = new QTcpSocket();
if (!mSocket->setSocketDescriptor(mSocketID))
{
emit socketError(mSocket->error());
}

mRequestEvent->setPeer(&mSocket->peerAddress());
mRequestEvent->setLocal(&mSocket->localAddress());

connect(mSocket, SIGNAL(readyRead()), this, SLOT(readyRead()), Qt::DirectConnection);
connect(mSocket, SIGNAL(disconnected()), this, SLOT(disconnected()), Qt::DirectConnection);

// report connection
emit(loginInitiated(mRequestEvent));
qint64 msgLength;

while (!mQuit)
{
mSocket->waitForReadyRead(1);
if (mResponse != NULL)
{
msgLength = mSocket->write(mResponse);
mSocket->flush();
NetEventInfo responseInfo = new NetEventInfo(QDateTime::currentDateTime(), &mSocket->localAddress(), &mSocket->peerAddress(), BuilderCore::NetMessageTypes::loginResult);
emit (loginFinalized(responseInfo));
delete mResponse;
mResponse = NULL;
}
}

// start thread
//exec();
}
@

[primem0ver]

Ok... I have played around with various things trying to figure out what is wrong... and I noticed something VERY odd...

If I use readAll like I did above it fails.
If I try and use the following modification it also fails. But if I use the COMMENTED LINE instead of the one commented current it works fine.

@
void LoginConnection::readyRead()
{
bool isReadable = mSocket->isReadable(); // always true
qint64 messageLength = mSocket->bytesAvailable();
QByteArray data = mSocket->read(messageLength); // CURRENT: fails with same error
//QByteArray data = mSocket->read(messageLength - 1); // THIS LINE WORKS IF UNCOMMENTED...
emit loginReceived(data);
}
@

BASICALLY... I cannot read the last byte of the message sent...

[SGaist]

Hi,

Pretty strange, what version of Qt/OS are you using ?

Also, please take a look at Qt 5's latest documentation about the use of QThread. You seem to go on the path of using it in a wrong way

[primem0ver]

I am using Qt 5.2 on Windows (currently 32 bit).

bq. Also, please take a look at Qt 5’s latest documentation about the use of QThread. You seem to go on the path of using it in a wrong way

I found an article that suggests the same thing (even though it is a rather different situation). I am going to look into that possibility. However, I have gone back to the original way I set this up (the code above follows a second example I found and tried because the first way was having the same problem even though it seemed more correct in terms of threading).

I have been using the workaround implied above by reading all but the last byte and then "peeking" the remaining byte into the QByteArray. It has worked until I try closing the connection. Then I get the same error as I do when trying to read the last byte.

[primem0ver]

I would also like to add this important fact:

I described in another topic where i was having a different problem that I am having to test the client and server components of this application within the same executable. (In a real world application/use of this program this will not generally occur). The client and server are in separate dlls that are loaded and used depending on the way the program is set up for use on a particular network node.

Here is what is important in regards to the comments above:
While the server's socket IS multithreaded, the client's tcp socket is NOT. Yet I am getting the same exact problem on the client socket instance. So that makes me wonder if threading is really the issue here. When I close the connection on the client side, it also causes the same crash to occur. (The server side does not actually close the thread).

[SGaist]

Are you getting the same kind of problem using the Threaded Fortune Server and Client example ?

[primem0ver]

Well... one significant issue with this is that neither one of those examples use the .ReadAll() function. I will test them anyway and see what happens but i don't think its a good test for my server since the fortune server doesn't read anything from the client. Mine does and responds accordingly. Which is why I didn't model my client/server after them.

[primem0ver]

The fortune client and server run. I have also modeled the code that reads data that comes over the socket after the fortune cookie reading method. It has not solved the problem. I still get the same issue on both the client and the server.

EDIT: There is one thing that is different between the fortune cookie reading method and my method. The data sent is NOT guaranteed to be a string as it is in the fortune cookie example so I cannot assume that it is a string. If I try reading it as a string, it may terminate early. Especially because string data is written differently than raw binary data. So I can only read it as such.

I also created a new class to streamline the process and to mimic both the server and client tcp socket reading and writing methods in the fortune cookie example within one class.

Here is my version of the read method:

@
void BidirectionalTcpConnection::readStream()
{
QDataStream in(&mSocket);
in.setVersion(QDataStream::Qt_5_2);
int n = mSocket.bytesAvailable();

if (mIncomingLength == 0)
{
if (mSocket.bytesAvailable() < (qint64)sizeof(qint64))
return;

in >> mIncomingLength;
}

if (mSocket.bytesAvailable() < mIncomingLength)
return;

char* buffer = new char[mIncomingLength];
int bytes = in.readRawData(buffer, mIncomingLength); // THIS LINE NOW CAUSES THE CRASH
//in >>

if (bytes == -1)
{
// throw an error...
}

QByteArray* incomingMessage = new QByteArray();
incomingMessage->append(buffer, mIncomingLength);
mIncomingMessageQueue.enqueue(incomingMessage);
emit(messageReceived());

mIncomingLength = 0;
}
@

[primem0ver]

I think I got it working now. The problem in this version was that the QDataStream.writeBytes(const char*, uint) which I used to create the "sent" message was putting in extra data I wasn't expecting. So the length read in on the stream and the actual number of bytes in the stream didn't agree. There were four bytes that were missing from the stream.

[SGaist]

Nice you found out a solution !

Did you set the QDataStream version on both end to the same value ?

[primem0ver]

I still think it is disturbing that a way that is meant to work doesn't. I originally ignored the fortune server/client example because it didn't serve the purposes I need. Especially when others had examples that were more appropriate. There are people out there who suggest the ReadAll method and the fact that it causes problems is an issue that I feel needs to be addressed.

But using the method suggested by the server/client example is a little more flexible in the long run. Particularly in the way it demonstrates handling a continuous stream in a way that keeps data segmented the way it was meant to be so I decided to use it.

I am not exactly sure what you mean by "value." Do you mean Qt version? If so... yes.