QTcpSocket - memory allocation problem

[dreac]

Hi All,

I have a problem with Qt's sockets. I have a server application that listens for incoming connections and a client application that connects to the host. So far so good. I'm able to create the connection and send/receive data. The client sends a huge amount of data (in blocks) to the server, while keeping up the connection.

I encountered a problem that drives me crazy since a few days now, and I haven't found any solution yet. I tried to narrow down the problem to minimum and to have an example code for showing.

At some point there is a memory issue, but I can't figure out what causes it:
std::bad_alloc at memory location 0x0019d434

Here is the code example

Server Application:

TcpServer


#ifndef TCPSERVER_H
#define TCPSERVER_H

#include "serversocket.h"

#include <QTcpServer>
#include <QHostAddress>

class TcpServer : public QTcpServer
{
Q_OBJECT
public:
TcpServer ( QObject * parent = 0 ) : QTcpServer(parent)
{
if (!this->listen( QHostAddress::LocalHost, 2688))
{
exit(-1);
}
};

protected:
virtual void incomingConnection ( int socketDescriptor )
{
ServerSocket *socket = new ServerSocket(this);
socket->setSocketDescriptor(socketDescriptor);
connect(socket, SIGNAL(readyRead()), socket, SLOT(readBlockData()));
};

};

#endif // TCPSERVER_H


ServerSocket


#ifndef SERVERSOCKET_H
#define SERVERSOCKET_H

#include <QTcpSocket>
#include <QHostAddress>
#include <QDataStream>

#include <iostream>
#include <cstring>

class ServerSocket : public QTcpSocket
{
Q_OBJECT
public:
ServerSocket ( QObject * parent = 0 ) : QTcpSocket(parent)
{
this->blockSize = 0;
};

public slots:
void readBlockData()
{
QDataStream in(this);
in.setVersion(QDataStream::Qt_4_7);

if (this->blockSize == 0)
{
if (this->bytesAvailable() < (int)sizeof(quint32))
return;

in >> this->blockSize;
}

if (this->bytesAvailable() < this->blockSize)
return;

char* blockData = new char[this->blockSize];
in >> blockData;

QByteArray byteArray(blockData);

std::cout << (int)byteArray.size() << " - " << this->bytesAvailable() << "\n";

this->blockSize = 0;

delete[] blockData;
};

private:
quint32 blockSize;


};

#endif // SERVERSOCKET_H


Main


#include <QtCore/QCoreApplication>

#include "tcpserver.h"

int main(int argc, char *argv[])
{
QCoreApplication a(argc, argv);

TcpServer *server = new TcpServer();

return a.exec();
}


Client application:

ClientSocket


#ifndef CLIENTSOCKET_H
#define CLIENTSOCKET_H

#include <QTcpSocket>
#include <QHostAddress>
#include <QDataStream>

#include <iostream>
#include <cstring>

class ClientSocket : public QTcpSocket
{
Q_OBJECT
public:
ClientSocket ( QObject * parent = 0 ) : QTcpSocket(parent)
{
connectToHost(QHostAddress::LocalHost, 2688);
if (!this->waitForConnected(3000))
exit(-1);
};

void writeBlockData(const char* blockData, size_t size)
{

QByteArray dataArray((char*) blockData, size);

QByteArray block;
QDataStream out(&block, QIODevice::WriteOnly);
out.setVersion(QDataStream::Qt_4_7);

out << (quint32)0;
out << dataArray;
out.device()->seek(0);
out << (quint32)(block.size() - sizeof(quint32));

this->write(block);
this->flush();

this->waitForBytesWritten(-1);
};
};

#endif // CLIENTSOCKET_H


Main



#include <QtCore/QCoreApplication>

#include "clientsocket.h"

int main(int argc, char *argv[])
{
QCoreApplication a(argc, argv);

unsigned int size = 1000*1000*3;
char *data = new char[size];

for (int i = 0; i < size; i++)
{
data[i] = 'q';
}

ClientSocket client;

for (int blocks = 0; blocks < 2000; blocks++)
{
client.writeBlockData(data, size);
std::cout << blocks << std::endl;
}

delete[] data;

return a.exec();
}


I was testing the code on Windows 7 with Qt 4.7 using visual studio 2008.

1.
The first problem is not really a problem, but more something that I found worth mentioning and I was wondering if someone else experienced something similar.
When running the code in Debug mode after reading data from the socket server, the buffer seems not to be cleared. The memory keeps increasing until there is nothing left to be allocated. This seems not to be the case for compiling and running the code in Release mode.

2.
But weird is, that even in Release mode, after the same number of received blocks, I get the above mentioned exception and the server application crashes.


Thank you!!!

[tbscope]

virtual void incomingConnection ( int socketDescriptor )
{
ServerSocket *socket = new ServerSocket(this);
socket->setSocketDescriptor(socketDescriptor);
connect(socket, SIGNAL(readyRead()), socket, SLOT(readBlockData()));
};
The memory keeps increasing until there is nothing left to be allocated. This seems not to be the case for compiling and running the code in Release mode.


Try to understand the piece of code I quoted above to see if you can spot the huge memory leak.
You are getting out of memory. In debug mode a lot faster than in release mode of course.

[dreac]

Thanks, but ... sorry, I don't get it. I can't see anything wrong with this. Could you please point out what's causing a memory leak here?

This is executed every time a client calls connectToHost(). Thus if only 1 client connects, then I will get only 1 instance of serversocket. The socket descriptor is passed to the tcp socket who then emits the signal readyRead() whenever data is coming in. The signal is connected to my method readBlockData() which reads the data from the stream, and thus should free the buffer.

I set a breakpoint at these lines to check how often a ServerSocket is instantiated, and it was only once as expected.

Or did I miss anything else here.

[tbscope]

ServerSocket *socket = new ServerSocket(this);
This creates a local variable called socket which points to a memory location.

Two important facts:
1. local means that the variable will be deleted at the end of the scope, in this case the } bracket of the function.
2. Since it is a pointer, the memory allocated will not be freed at the end of the scope.

This has a consequence:
The allocated memory still exists, but you can't access it anymore (the socket variable).

What you need to do:
Manage all your pointers.
If there isn't a parent/child relationship where the parent frees all the child allocations, or in the case the lifetime of the parent is too long (as in the parent exists for the duration the program is being run), then you need to explicitly free the resources yourself.

Thus:
In your case, just before the scope of the function ends, you should delete the memory allocated. This, of course, will make your program not work anymore like you expect it.

Thus:
You should really define your pointers at class level so they are accessible everywhere in the class for the lifetime of an object based on that class.

[dreac]

Thank you again for your quick reply.

Well, I see your point in this. I agree that this isn't clean and it should be solved in a smarter way.

If you replace the TcpServer class with the following code then you don't lose access to 'socket' as it is a private member now, yet the problem remains. Also, in this case you can be sure that no more than 1 connection is open. Of course, this doesn't make much sense but serves as demonstration purpose only.



class TcpServer : public QTcpServer
{
Q_OBJECT
public:
TcpServer ( QObject * parent = 0 ) : QTcpServer(parent)
{
this->socket = NULL;

if (!this->listen( QHostAddress::LocalHost, 2688))
{
exit(-1);
}
};

protected:
virtual void incomingConnection ( int socketDescriptor )
{
if (this->socket == NULL)
{
this->socket = new ServerSocket(this);
socket->setSocketDescriptor(socketDescriptor);
connect(socket, SIGNAL(readyRead()), socket, SLOT(readBlockData()));
}
};

private:
ServerSocket *socket;

};




Edit:
I found the solution to my problem. Here it is. It was in the method readBlockData(). I commented out the line that caused problems and added the new lines. It was something with the QDataStream. My guess ... maybe something about null terminated data.



void readBlockData()
{
QDataStream in(this);
in.setVersion(QDataStream::Qt_4_7);

if (this->blockSize == 0)
{
if (this->bytesAvailable() < (int)sizeof(quint32))
return;

in >> this->blockSize;
}

if (this->bytesAvailable() < this->blockSize)
return;

char* blockData = new char[this->blockSize];

// these lines caused the memory leak for whatever reason
// in >> blockData;
// QByteArray byteArray(blockData);

// new lines
int bytesRead = in.readRawData(blockData, this->blockSize);

if (bytesRead != this->blockSize)
{
// do something here ...
}

QByteArray byteArray(blockData, bytesRead);
// end new lines

std::cout << (int)byteArray.size() << " - " << this->bytesAvailable() << "\n";

this->blockSize = 0;

delete[] blockData;
};


Thanks tbscope for your input :)