Server crash

Idodoqdo

Hi all. Faced such a problem. When a client connects, I create a new socket and put in a vector of it. But when it is disconnected, this socket still remains in the vector, although it should be deleted via deleteLater (probably?). If I reconnect as a client and try to send a message, then it duplicates when debugging, and when it starts up normally, it crashes. help me please
Server.cpp

Server::Server()
{
    if (listen(QHostAddress::Any, 2323)) {
        qDebug() << "start";
    } else {
        qDebug() << "error";
    }
    next_block_size_ = 0;
}

void Server::SendToClient(QString str)
{
    data_.clear();
    QDataStream out(&data_, QIODevice::WriteOnly);
    out.setVersion(QDataStream::Qt_6_2);
    out << quint16(0) << str;
    out.device()->seek(0);
    out.device()->seek(data_.size() - sizeof(quint16));
    for (int i = 0; i < Sockets.size(); ++i) {
        Sockets[i]->write(data_);
    }

}


void Server::incomingConnection(qintptr socketDescriptor)
{   qDebug() << "hello";
    socket = new QTcpSocket;
    socket->setSocketDescriptor(socketDescriptor);
    connect(socket, &QTcpSocket::readyRead, this, &Server::slotReadyRead);
    connect(socket, &QTcpSocket::disconnected, socket, &QTcpSocket::deleteLater);
    Sockets.push_back(socket);
    qDebug() << "client connected" << socketDescriptor;
}

void Server::slotReadyRead() {
    socket = (QTcpSocket*)sender();
    QDataStream in(socket);
    in.setVersion(QDataStream::Qt_6_2);
    if (in.status() == QDataStream::Ok) {
        qDebug() << "read ...";
        while(1) {
            if (next_block_size_ == 0) {
                if (socket->bytesAvailable() < 2) {
                    break;
                }
                in >> next_block_size_;
            }
            if (socket->bytesAvailable() < next_block_size_) {
                break;
            }
            QString str;
            in >> str;
            next_block_size_ = 0;
            SendToClient(str);
            break;
        }

    } else {
        qDebug() << "Error DataStream";
    }

}

MainWindow.cpp(client)

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
    , ui(new Ui::MainWindow)
{
    ui->setupUi(this);
    socket = nullptr;
    connect(ui->ConnectToServer, SIGNAL(released()), this, SLOT(Connect()));
}

MainWindow::~MainWindow()
{
    delete ui;
}


void MainWindow::Connect()
{
    if (socket == nullptr) {
    socket = new QTcpSocket(this);
    connect(socket, &QTcpSocket::readyRead, this, &MainWindow::slotReadyRead);
    connect(socket, &QTcpSocket::disconnected, socket, &QTcpSocket::deleteLater);
    qDebug() << "press connect";
    next_block_size_ = 0;
    socket->connectToHost("127.0.0.1", 2323);
    if(!socket->waitForConnected(5000))
    {

        qDebug() << "Error: " << socket->errorString();
        delete socket;
        socket = nullptr;
    }
    }

}

void MainWindow::SendToServer(QString str)
{
    if (socket != nullptr) {
    data_.clear();
    QDataStream out(&data_, QIODevice::WriteOnly);
    out.setVersion(QDataStream::Qt_6_2);
    out << quint16(0) << str;
    out.device()->seek(0);
    out.device()->seek(data_.size() - sizeof(quint16));
    socket->write(data_);
    ui->lineEdit->clear();
    }
}

void MainWindow::slotReadyRead()
{
    QDataStream in (socket);
    in.setVersion(QDataStream::Qt_6_2);
    if (in.status() == QDataStream::Ok) {
        while(1) {
            if (next_block_size_ == 0) {
                if (socket->bytesAvailable() < 2) {
                    break;
                }
                in >> next_block_size_;
            }
            if (socket->bytesAvailable() < next_block_size_) {
                break;
            }
            QString str;
            in >> str;
            next_block_size_ = 0;
            ui->textBrowser->append(str);
        }
    } else {
        ui->textBrowser->append("Error read");
    }
}


void MainWindow::on_pushButton_2_clicked()
{
    SendToServer(ui->lineEdit->text());
}


void MainWindow::on_lineEdit_returnPressed()
{
    SendToServer(ui->lineEdit->text());
}

sierdzio

Your vector holds pointers to sockets. So when a socket is deleted, the object is gone, but the pointer is still there (this is called a "dangling pointer"). You should manually remove the pointer from the vector when socket is deleted.

For example like this:

connect(socket, &QTcpSocket::disconnected, this, [this]() { 
  auto socket = qobject_cast<QTcpSocket*>(sender());
  if (socket) {
    Sockets.remove(socket); // Or erase() or whatever you are using
    socket->deleteLater();
  } else {
    // Error handling
  }  
});

You should also guard against sending to dangling pointers in your sender code:

for (int i = 0; i < Sockets.size(); ++i) {
  auto socket = QPointer<QTcpSocket>(Sockets.at(i));
  if (socket) {
      socket->write(data_);
  } else {
    // Remove from vector?
  }
}

Idodoqdo

connect(socket, &QTcpSocket::disconnected, this, [this]()

Error: expected expression

Idodoqdo

Sockets.remove(socket);

but there is no such overload

jsulm

@Idodoqdo said in Server crash:

Error: expected expression

Please post the whole connect statement you have now

jsulm

@Idodoqdo said in Server crash:

but there is no such overload

you can use https://doc.qt.io/qt-5/qvector.html#removeAll

Idodoqdo

@jsulm
now I do like this, but for some reason it is not removed from QMap :(

Server::Server()
{
    if (listen(QHostAddress::Any, 2323)) {
        qDebug() << "start";
    } else {
        qDebug() << "error";
    }
    next_block_size_ = 0;
}

void Server::SendToClient(QString str)
{
    data_.clear();
    QDataStream out(&data_, QIODevice::WriteOnly);
    out.setVersion(QDataStream::Qt_6_2);
    out << quint16(0) << str;
    out.device()->seek(0);
    out.device()->seek(data_.size() - sizeof(quint16));
    for (auto i = Sockets.begin(); i != Sockets.end(); ++i) {
        if (i.value()) {
            i.value()->write(data_);
        } else {
            Sockets.remove(i.key());
        }
    }
}


void Server::incomingConnection(qintptr socketDescriptor)
{   qDebug() << "hello";
    socket = new QTcpSocket;
    socket->setSocketDescriptor(socketDescriptor);
    connect(socket, &QTcpSocket::readyRead, this, &Server::slotReadyRead);
    connect(socket, &QTcpSocket::disconnected, this, &Server::Disconnect);
    Sockets.insert(socket->socketDescriptor(), socket);
    qDebug() << "client connected" << socketDescriptor;
}

void Server::slotReadyRead() {
    socket = (QTcpSocket*)sender();
    QDataStream in(socket);
    in.setVersion(QDataStream::Qt_6_2);
    if (in.status() == QDataStream::Ok) {
        qDebug() << "read ...";
        while(1) {
            if (next_block_size_ == 0) {
                if (socket->bytesAvailable() < 2) {
                    break;
                }
                in >> next_block_size_;
            }
            if (socket->bytesAvailable() < next_block_size_) {
                break;
            }
            QString str;
            in >> str;
            next_block_size_ = 0;
            SendToClient(str);
            break;
        }

    } else {
        qDebug() << "Error DataStream";
    }

}

void Server::Disconnect()
{
    auto socket = qobject_cast<QTcpSocket*>(sender());
    if (socket) {
        int kek = 0;
        kek = Sockets.remove(socket->socketDescriptor());
        socket->deleteLater();
    }
}

sierdzio

OK so it is a QMap. I didn't know what container you have used so I wrote remove with a comment to guide you to look for proper removal method.

Your code looks roughly OK now, should work as long as this trick with socket descriptors is valid. But really you can use just a QVector or QList to store pointers to sockets and then call removeOne() or removeAll().

Idodoqdo

@sierdzio
Thanks a lot. It helped me, but if I have to change the code in the future so that the clients have the functionality, let's say: 1) Enter a name 2) Write a message
So how do I process these requests from the respective user?