QList of QTcpSocket derived class pointers (stack corruption)

[zAAm]

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);
	}	
}

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);
	}	
}

To copy to clipboard, switch view to plain text mode 

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"));
}

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"));
}

To copy to clipboard, switch view to plain text mode 

Thanks for any suggestions

[zAAm]

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;

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

To copy to clipboard, switch view to plain text mode 

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;

QList<ClientSocket> clients;

To copy to clipboard, switch view to plain text mode 

Or is this just wasting memory?

[zAAm]

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)

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]

Any ideas? Or something I could use to find where the heap corruption happens?

[zAAm]

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

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]

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...

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