QTcpServer crashes/freezes after some time

thorsten2102

Hey,

i made a little server with QTcpServer and QTcpSocket running as a Console Application and a client with QTcpSocket running on android.

The whole server runs for some time (5 - 60 min.). The Client application so far seems not be the problem, as this one works just normal. After some time the server either crashes and the application closes without any kind of message/report or it does not accept new connections anymore and does not create a new client object.

I'm able to connect multiple clients at the same time and disconnect and connect them a few times + send data in both directions, until the server freezes at some point.

If the server just freezes and the console application is still open, i can press CTRL + C once which would normally close the application, in this case it works again and "unfreezes". When its running correctly and i hit CTRL + C again, the application closes normally.

server.cpp

Server::Server(char* argvIn[])
    : pathWordList("C:\\Users\\Administrator\\Desktop\\wordListServer\\lists\\wordlist.txt"),
      pathFinalWordList("C:\\Users\\Administrator\\Desktop\\wordListServer\\lists\\finalwordlist.txt"),
      pathSortedOut("C:\\Users\\Administrator\\Desktop\\wordListServer\\lists\\sortedout.txt"),
      pathUserSuggested("C:\\Users\\Administrator\\Desktop\\wordListServer\\lists\\usersuggested.txt"),
      pathBackup("C:\\Users\\Administrator\\Desktop\\wordListServer\\backups")
{
    loadLists();

    server = new QTcpServer();
    server->listen(QHostAddress::AnyIPv4, 59608);
    connect(server, SIGNAL(newConnection()), this, SLOT(newClient()));

    std::cout << "listening on port: 59608\n";

    QTimer *timer = new QTimer(this);
    connect(timer, SIGNAL(timeout()), this, SLOT(saveLists()));
    timer->start(3600000); //21600000 msec = 6 hours
}

Server::~Server()
{

}

void Server::newClient()
{
    cl = new Client(server->nextPendingConnection(), &wordList, &finalWordList, &sortedOut, &userSuggested);
    std::cout << "new client connected\n";
}

server.h

class Server : public QObject
{
    Q_OBJECT
public:
    Server(char* argvIn[]);
    ~Server();

private slots:
    void newClient();
    void saveLists();

private:
    void loadLists();

private:
    QTcpServer* server;
    Client* cl;
    QStringList wordList;
    QStringList finalWordList;
    QStringList sortedOut;
    QStringList userSuggested;

    QString pathWordList;
    QString pathFinalWordList;
    QString pathSortedOut;
    QString pathUserSuggested;
    QString pathBackup;
};

client.cpp

Client::Client(QTcpSocket* socketIn, QStringList* wordListIn, QStringList* finalWordListIn, QStringList* sortedOutIn, QStringList* userSuggestedIn)
    : socket(socketIn),
      wordList(wordListIn),
      finalWordList(finalWordListIn),
      sortedOut(sortedOutIn),
      userSuggested(userSuggestedIn)

{
    rng = new QRandomGenerator(static_cast<quint32>(QDateTime::currentMSecsSinceEpoch()));

    connect(socket, SIGNAL(readyRead()), this, SLOT(readData()));
    connect(socket, SIGNAL(disconnected()), this, SLOT(connectionLost()));
}

Client::~Client()
{

}

void Client::connectionLost()
{
    std::cout << "client disconnected\n";
    socket->close();
    socket = nullptr; //if i don't set it to nullptr the server crashes when disconnecting
    delete socket;
    delete this;
}

client.h

class Client : public QObject
{
    Q_OBJECT
public:
    Client(QTcpSocket* socketIn, QStringList* listIn, QStringList* finalWordListIn, QStringList* sortedOutIn, QStringList* userSuggestedIn);
    ~Client();

private slots:
    void readData();
    void connectionLost();

private:
    QTcpSocket* socket;
    QStringList* wordList;
    QStringList* finalWordList;
    QStringList* sortedOut;
    QStringList* userSuggested;
    QByteArray buffer;
    QRandomGenerator* rng;

};

The server saves all data every hour and when only few clients are connecting like just 1 client once for 2 min. it runs for several hours and does its regular backups.

I assume the client is not being deleted correctly and i may have a memory leak. I'm not completely sure about this.

aha_1980

@thorsten2102

void Client::connectionLost()
{
std::cout << "client disconnected\n";
socket->close();

Your slot is called when the connection is already closed, so closing it again here makes no sense.

If it would not be closed, and you call close() immediately followed from delete, then your socket would not do what you expect it to do.

socket = nullptr; //if i don't set it to nullptr the server crashes when disconnecting
delete socket;
delete this;

try to replace these lines with:

socket->deleteLater()
deleteLater()

Also, in your server you assign new clients to the member variable Client* cl;, which means on every client connection you overwrite the last one. Probably not what you expect.

By the way, passing QStringList* wordListIn does not look correct to me. Try const QStringList &wordListIn for read-only lists.

Regards

aha_1980

@thorsten2102 said in QTcpServer crashes/freezes after some time:

socket = nullptr; //if i don't set it to nullptr the server crashes when disconnecting
delete socket;

That is wrong. The correct order is:

delete socket;
socket = nullptr;

delete this;

What is that? Deleting yourself doesn't look right.

Regards

thorsten2102

@aha_1980 i have changed the order as you stated

void Client::connectionLost()
{
    std::cout << "client disconnected\n";
    socket->close();
    delete socket;
    socket = nullptr;
    delete this;
}

now the server instantly crashes when a client disconnects.

about the "delete this" i somehow have/want to destroy that client object when the client disconnects

edit:

delete this;

seems to be allowed https://isocpp.org/wiki/faq/freestore-mgmt#delete-this

aha_1980

@thorsten2102

void Client::connectionLost()
{
std::cout << "client disconnected\n";
socket->close();

Your slot is called when the connection is already closed, so closing it again here makes no sense.

If it would not be closed, and you call close() immediately followed from delete, then your socket would not do what you expect it to do.

socket = nullptr; //if i don't set it to nullptr the server crashes when disconnecting
delete socket;
delete this;

try to replace these lines with:

socket->deleteLater()
deleteLater()

Also, in your server you assign new clients to the member variable Client* cl;, which means on every client connection you overwrite the last one. Probably not what you expect.

By the way, passing QStringList* wordListIn does not look correct to me. Try const QStringList &wordListIn for read-only lists.

Regards

thorsten2102

@aha_1980

@aha_1980 said in QTcpServer crashes/freezes after some time:

@thorsten2102

void Client::connectionLost()
{
std::cout << "client disconnected\n";
socket->close();

Your slot is called when the connection is already closed, so closing it again here makes no sense.

If it would not be closed, and you call close() immediately followed from delete, then your socket would not do what you expect it to do.
socket = nullptr; //if i don't set it to nullptr the server crashes when disconnecting
delete socket;
delete this;
try to replace these lines with:
socket->deleteLater()
deleteLater()

yeah, the event loop is still running, so this was the problem. This fixed it.

Also, in your server you assign new clients to the member variable Client* cl;, which means on every client connection you overwrite the last one. Probably not what you expect.

the thing is, multible connections at the same time do work in this application. So when i create a new Client object the server "forgets" the last one right? i think this is not a big issue in this case, as the client object destroys itself, but what would be the proper way to do it? I've seen people doing it with lists or vectors, but when a client objects destroys itself, what do i do with the disconnected clients in the list/vector. or whats the proper way to do it?

By the way, passing QStringList* wordListIn does not look correct to me. Try const QStringList &wordListIn for read-only lists.

Regards

in this application i also want to read and write in these lists from the client. Would passing it by reference be better than by pointers?
like

QStringList &wordListIn

thanks for your help. and Merry Christmas.

aha_1980

@thorsten2102 said in QTcpServer crashes/freezes after some time:

the thing is, multible connections at the same time do work in this application. So when i create a new Client object the server "forgets" the last one right? i think this is not a big issue in this case, as the client object destroys itself, but what would be the proper way to do it? I've seen people doing it with lists or vectors, but when a client objects destroys itself, what do i do with the disconnected clients in the list/vector. or whats the proper way to do it?

You could replace the member variable with a local variable, for example. That would at least make a bit clearer that you don't care for this pointer after the function.

in this application i also want to read and write in these lists from the client. Would passing it by reference be better than by pointers?

Ah, ok. In that case I'd indeed prefer pointers (as Qt does) because on caller side you see that the object is passed as pointer and not as value. But you can use references also, there is no correct style here.

Merry Christmas!