Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-66230

QTcpSocket drops payload when two writes and a close overlap

    XMLWordPrintable

Details

    • Bug
    • Resolution: Out of scope
    • P2: Important
    • None
    • 5.10.0, 5.11
    • Network: Sockets
    • None
    • Tested on macOS 10.13 and linux, suspected to happen at least also on macOS 10.12 and on windows.

    Description

      Consider

      #include <QCoreApplication>
      #include <QTcpSocket>
      #include <QTcpServer>
      #include <QTimer>
      #include <QProcess>
      #include <QElapsedTimer>
      
      void writeSocket(QTcpSocket *socket, qint64 *count)
      {
          const int bytesToWrite = (*count + 1) % 1024 + 1;
          const qint64 written = socket->write(QByteArray(bytesToWrite, 'x'));
          if (written < 0)
              qDebug() << "write error";
          else
              *count += written;
      };
      
      void readSocket(QTcpSocket *socket, qint64 *count)
      {
          const qint64 toSkip = socket->bytesAvailable();
          const qint64 skipped = socket->skip(toSkip);
          if (skipped < 0)
              qDebug() << "read error";
          else
              *count += skipped;
      };
      
      void initSocket(QTcpSocket *socket, qint64 *readCount, qint64 *writeCount)
      {
          QObject::connect(socket, &QAbstractSocket::bytesWritten, [=]() {
              writeSocket(socket, writeCount);
          });
          QObject::connect(socket, &QAbstractSocket::readyRead, [=]() {
              readSocket(socket, readCount);
              QElapsedTimer timer;
              timer.start();
              // Busy wait to block the event loop a bit.
              while (timer.elapsed() < (*readCount % 16));
          });
      };
      
      
      int server(int argc, char **argv)
      {
          const int port = QByteArray(argv[2]).toInt();
      
          qint64 socketWritten = 0;
          qint64 socketRead = 0;
      
          QCoreApplication a(argc, argv);
          QTcpServer server;
      
          QScopedPointer<QTcpSocket> socket;
          QObject::connect(&server, &QTcpServer::newConnection, [&]() {
              socket.reset(server.nextPendingConnection());
              initSocket(socket.data(), &socketRead, &socketWritten);
              writeSocket(socket.data(), &socketWritten);
              QTimer::singleShot(100, socket.data(), [&](){
                  QObject::disconnect(socket.data(), &QAbstractSocket::bytesWritten,
                                      nullptr, nullptr);
                  while (socket->bytesToWrite() > 0) {
                      if (!socket->waitForBytesWritten())
                          qDebug() << "ouch" << socket->bytesToWrite();
                  }
      
                  a.quit();
              });
          });
      
          server.listen(QHostAddress::LocalHost, port);
          const int exitCode = a.exec();
      
          qDebug() << socketWritten << socketRead;
          return exitCode;
      }
      
      int client(int argc, char **argv)
      {
          const int port = QByteArray(argv[2]).toInt();
      
          qint64 socketWritten = 0;
          qint64 socketRead = 0;
      
          bool wasConnected = false;
      
          QCoreApplication a(argc, argv);
          QScopedPointer<QTcpSocket> socket(new QTcpSocket);
          QObject::connect(socket.data(), &QTcpSocket::connected, [&]() {
              wasConnected = true;
              writeSocket(socket.data(), &socketWritten);
          });
      
          QObject::connect(socket.data(),
                           QOverload<QAbstractSocket::SocketError>::of(&QAbstractSocket::error), [&]() {
              if (!wasConnected)
                  socket->connectToHost(QHostAddress::LocalHost, port);
          });
      
          QObject::connect(socket.data(), &QAbstractSocket::stateChanged, [&]() {
              if (wasConnected) {
                  if (socket->bytesAvailable() > 0)
                      qDebug() << "ouch" << socket->bytesAvailable();
                  if (socket->state() == QAbstractSocket::UnconnectedState) {
                      a.quit();
                  }
              }
          });
      
          initSocket(socket.data(), &socketRead, &socketWritten);
          socket->connectToHost(QHostAddress::LocalHost, port);
      
          const int exitCode = a.exec();
          qDebug() << socketRead << socketWritten;
          return exitCode;
      }
      
      
      int main(int argc, char **argv)
      {
          if (argc > 1) {
              if (strcmp(argv[1], "server") == 0)
                  return server(argc, argv);
              else if (strcmp(argv[1], "client") == 0)
                  return client(argc, argv);
          } else {
              int serverRead, serverWritten, clientRead, clientWritten;
      
              QCoreApplication a(argc, argv);
              int port = 3345;
              do {
                  QProcess server;
                  QProcess client;
      
                  server.setProcessChannelMode(QProcess::MergedChannels);
                  client.setProcessChannelMode(QProcess::MergedChannels);
      
                  QString portString = QString::number(++port);
                  server.start(QCoreApplication::applicationFilePath(), QStringList({"server", portString}));
                  client.start(QCoreApplication::applicationFilePath(), QStringList({"client", portString}));
      
                  QObject::connect(&server, QOverload<int>::of(&QProcess::finished), [&](int exitCode) {
                      QByteArrayList lines = server.readAll().trimmed().split('\n');
                      QByteArrayList serverOut = lines.last().trimmed().split(' ');
                      qDebug() << "server" << exitCode << lines;
                      serverWritten = serverOut[0].toInt();
                      serverRead = serverOut[1].toInt();
      
                      if (client.state() == QProcess::NotRunning)
                          a.quit();
                  });
      
                  QObject::connect(&client, QOverload<int>::of(&QProcess::finished), [&](int exitCode) {
                      QByteArrayList lines = client.readAll().trimmed().split('\n');
                      QByteArrayList clientOut = lines.last().trimmed().split(' ');
                      qDebug() << "client" << exitCode << lines;
                      clientRead = clientOut[0].toInt();
                      clientWritten = clientOut[1].toInt();
                      if (server.state() == QProcess::NotRunning)
                          a.quit();
                  });
      
                  a.exec();
              } while (serverWritten == clientRead);
      
              return 0;
          }
      }
      

      The parent process should never terminate as the client doesn't initiate the connection drop and always reads all data available. The server makes sure all bytes are sent before it quits. Thus, the number of bytes read by the client should always be equal to the number of bytes written by the server.

      (except in the "ouch" cases, which can theoretically happen, but haven't been observed here. But as the "ouch" would be printed, that would be visible)

      However, there is code in QAbstractSocket and friends that prematurely closes the read notifier when failing to write to the socket. This failure to write can already happen on the client side before the regular TCP connection termination finishes because the OS already knows that the server has closed the socket. If the timing is such that the server writes a chunk of data, then immediately closes the connection, and the client tries to write to its socket before the data has arrived, the client will drop the read notification and never notice that further data arrives. That causes the data to be dropped.

      The relevant close() and and abort() calls are in QAbstractSocketPrivate::writeToSocket() and NativeSocketEnginePrivate::nativeWrite(), in the unix and win variants.

      I suspect a similar problem occurs when generating a read error on a socket that can still write. We call resetSocketLayer() in QAbstractSocketPrivate::readFromSocket() and QAbstractSocket::readData() in that case, which terminates the write notifier. Therefore we would never get notified of any remaining data being written.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              tpochep Timur Pocheptsov
              ulherman Ulf Hermann
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes