PDA

View Full Version : QList of QTcpSocket derived class pointers (stack corruption)



zAAm
19th April 2010, 11:24
Hi

I'm trying to build a streaming server/client program that accommodates multiple clients. I have to keep track of all of them at any one time to determine which one sent a message as ALL data has to be multiplexed through port 4012.

At the moment I'm allocating a new QTcpSocket object each time a new connection is made and keeping track of them through a QList object, but I keep getting heap corruption errors on destruction. I think it may be because of the wrong allocation of sockets or through the destruction of them.

I've been struggling with this for a while now and it's very frustrating as the errors aren't consistent.

Here's the code when QTcpServer signals newConnection(): (ClientSocket inherits from QTcpSocket)


void Server::acceptConnection()
{
//Allocate space for new connection
ClientSocket *client = new ClientSocket(this);
client = (ClientSocket*)server->nextPendingConnection();

if (!client)
QMessageBox::information(this, tr("Error"), tr("No client pointer allocated"));
//Seach for client in client list
int found = -1;

if (!clients.isEmpty()) {
for (int i = 0; i < clients.size(); ++i) {
if (clients.at(i)->peerAddress().toString() == client->peerAddress().toString())
found = i;
}
//If not found in array: Don't add duplicates ie. from same host
if (found == -1) {
clients.append(client);
++connectionCount;

updateConnectionLabel();
connect(clients.last(), SIGNAL(disconnected()), this, SLOT(clientDisconnected()));
connect(clients.last(), SIGNAL(readyRead()), this, SLOT(startRead()));
clients.last()->setOpenConnections(MAX_DATA_CLIENTS);
clients.last()->setReadBufferSize(READ_BUFFER_MAX);
} else {
//If already connected, add to data client list: Add duplicates to data client array
if (connectedClients < MAX_DATA_CLIENTS) {
dataClients.append(client);
++connectedClients;
updateConnectionLabel();
connect(dataClients.last(), SIGNAL(disconnected()), this, SLOT(clientDisconnected()));
}
}
} else {
//Add to array
clients.append(client);
++connectionCount;

updateConnectionLabel();
connect(clients.last(), SIGNAL(disconnected()), this, SLOT(clientDisconnected()));
connect(clients.last(), SIGNAL(readyRead()), this, SLOT(startRead()));
clients.last()->setReadBufferSize(READ_BUFFER_MAX);
clients.last()->setOpenConnections(MAX_DATA_CLIENTS);
}
}

The destruction code at this time (I've tried various methods):


void Server::clientDisconnected()
{
//Remove all the unconnected sockets from the client list
for (int i = 0; i < clients.size(); ++i) {
if (clients.at(i)->state() == QAbstractSocket::UnconnectedState)
{
clients.at(i)->close();
clients.at(i)->deleteLater();
clients.removeAt(i);

--connectionCount;
updateConnectionLabel();
}
}

//Remove unconnected data clients from dataclient list
for (int i = 0; i < dataClients.size(); ++i) {
if (dataClients.at(i)->state() == QAbstractSocket::UnconnectedState)
{
dataClients.at(i)->close();
dataClients.at(i)->deleteLater();
dataClients.removeAt(i);

--connectedClients;
updateConnectionLabel();
}
}
QMessageBox::information(this, tr("DISCONNECT"), tr("DISCONNECT"));
}

Thanks for any suggestions :)

zAAm
19th April 2010, 18:39
I forgot to add the definition of clients and dataClients. ClientSocket inherits from QTcpSocket and adds just a few parameters like openConnections and so on.


QList<ClientSocket *> clients;
QList<ClientSocket *> dataClients;


Is this the way to go or is there a better way?

Should I rather use lists of actual objects than the pointers? Such as

QList<ClientSocket> clients;
Or is this just wasting memory?

zAAm
19th April 2010, 23:28
Is it possible that since the functions aren't thread-safe they are trying to access the same class member? (in this case the QList clients) :confused:

I know that disconnected() and readyRead() will be called in worker threads, so can that be cause of the heap corruption? When disconnected() is called while still in the readyRead() function for example and the socket is destroyed before the read function is completed?
Should I use QMutexLocker then?

zAAm
20th April 2010, 17:15
Any ideas? Or something I could use to find where the heap corruption happens? :confused:

zAAm
20th April 2010, 18:25
Ok, I think what I'm trying to do with the Client *client = (ClientSocket *)server->nextPendingConnection() line is to downcast from base to derived class. (Since ClientSocket is derived from QTcpSocket and nextPendingConnection returns a pointer to a QTcpSocket).
I've read that this is a bad thing, even when using pointers, as I'm abusing the whole inheritance system :p

I thought about using static_cast, but I guess it would be better to change it altogether by using a separate class ClientSocket that doesn't inherit from QTcpSocket but contains a QTcpSocket pointer? Also, would using auto pointers help in this regard?

zAAm
20th April 2010, 18:48
Ok, it was definitely the downcasting that caused the issues. I've basically rewritten the whole ClientSocket class to include a QTcpSocket pointer that I can access publicly. This solved the heap corruption errors... :cool:

Now to go through the rest of the 1500 lines and check that everything is as it should be with the new class... :rolleyes: