QTcpSocket sends data twice with flush()

[xenome]

Hi,

I have a strange problem where QTcpSocket sends my data twice if I call flush() following a write(). If I don't call flush the data is transmitted correctly and visible in my netcat capture. If I call flush, i see the data twice. The only time I don't have this problem while calling flush is when I wait an extended period of time before running my application a second time. I can't imagine my data is getting caught up in some OS buffer, but I have no idea what's going on. Anyone have an idea?

Thanks.

[xenome]

I figured it might be helpful to post the code. I've cut out most everything but a basic configuration.

I use netcat to capture the output and provide the server side socket. I'm piping data into netcat to trigger my client side socket write:

here's the command I use: nc -l -s 127.0.0.1 -p 7496 < zeros.txt

zeros.txt is a 60KB file filled with ascii 0's.

I'm using the 4.6.2 and this problem occurs on both Windows (qt creator/netcat with cygwin) and Linux.

Here's the header:

#ifndef INTERFACE_H
#define INTERFACE_H
 
#include <QtCore>
#include <QTcpSocket>
 
class Thread : public QThread
{
   Q_OBJECT
 
   public:
      Thread();
 
      // Thread entry point
      void run(void);
      qint32 tx(void);
 
   public slots:
      void multiplexIO(void);
 
   private:
      QTcpSocket* mp_Socket;
};
 
#endif

#ifndef INTERFACE_H
#define INTERFACE_H

#include <QtCore>
#include <QTcpSocket>

class Thread : public QThread
{
   Q_OBJECT

   public:
      Thread();
     
      // Thread entry point
      void run(void);
      qint32 tx(void);
   
   public slots:
      void multiplexIO(void);

   private:
      QTcpSocket* mp_Socket;
};

#endif

To copy to clipboard, switch view to plain text mode 

Here's the src:

#include "interface.h"
 
int reqData=1;
 
///////////////////////////////////////////////////////////////
Thread::Thread()
{
   start();
}
 
void Thread::multiplexIO(void)
{
   if(reqData)
   {
      qDebug() << "making request";
      reqData = 0;
      tx();
   }
}
 
void Thread::run(void)
{
   qDebug() << "thread started:" << currentThreadId();
 
   // Create TCP interface socket
   mp_Socket = new QTcpSocket;
   mp_Socket->connectToHost("127.0.0.1", 7496);
 
   // Wait for connection to be established
   if (!mp_Socket->waitForConnected(1000)) 
   {
      qDebug() << "Connect timed out: " << mp_Socket->errorString();
      return;
   }
 
   // Connect our socket read routine to our signal that new data has arrived
   //connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()));
   connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()), Qt::QueuedConnection);
 
   // Force the first call to check our socket
   // multiplexIO();
 
   // Start thread's event loop
   exec();
   qDebug() << "thread terminated";
}
 
qint32 Thread::tx()
{
   qDebug() << "Making data request!";
   if(!mp_Socket)
   {
      return -1;
   }
 
   mp_Socket->write("hello\n", 6);
   mp_Socket->flush();
   mp_Socket->waitForBytesWritten(10000);
   // mp_Socket->disconnectFromHost();
   return 0;
}
 
//////////////////
// Main
//////////////////
int main(int argc, char *argv[])
{
   QCoreApplication core(argc, argv);
   Thread thread;
   core.exec();
}

#include "interface.h"

int reqData=1;

///////////////////////////////////////////////////////////////
Thread::Thread()
{
   start();
}

void Thread::multiplexIO(void)
{
   if(reqData)
   {
      qDebug() << "making request";
      reqData = 0;
      tx();
   }
}

void Thread::run(void)
{
   qDebug() << "thread started:" << currentThreadId();

   // Create TCP interface socket
   mp_Socket = new QTcpSocket;
   mp_Socket->connectToHost("127.0.0.1", 7496);

   // Wait for connection to be established
   if (!mp_Socket->waitForConnected(1000)) 
   {
      qDebug() << "Connect timed out: " << mp_Socket->errorString();
      return;
   }

   // Connect our socket read routine to our signal that new data has arrived
   //connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()));
   connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()), Qt::QueuedConnection);

   // Force the first call to check our socket
   // multiplexIO();
   
   // Start thread's event loop
   exec();
   qDebug() << "thread terminated";
}

qint32 Thread::tx()
{
   qDebug() << "Making data request!";
   if(!mp_Socket)
   {
      return -1;
   }
   
   mp_Socket->write("hello\n", 6);
   mp_Socket->flush();
   mp_Socket->waitForBytesWritten(10000);
   // mp_Socket->disconnectFromHost();
   return 0;
}

//////////////////
// Main
//////////////////
int main(int argc, char *argv[])
{
   QCoreApplication core(argc, argv);
   Thread thread;
   core.exec();
}

To copy to clipboard, switch view to plain text mode 

[norobro]

I have no idea what the problem is but your code works on my Debian Sid box. I get "hello" once in a terminal flush() or no flush().

[xenome]

Did you run it several times? I found that I had to do it 3-10 times.

My procedure is:
start netcat
start app
kill netcat
restart netcat
kill app
restart app

[norobro]

Did your procedure ten times and never saw "hello" more than once.

[xenome]

Ugh!

Well that's strange, i can replicat this on windows and linux.

I'm trying to code up a quick QTcpServer interface to test this w/o netcat, but I get an error that I can't create a child in a different thread from the parent:

QTcpSocket* NextSock = mp_Server->nextPendingConnection();
   NextSock->write("p", 1);
   NextSock->flush();
   NextSock->waitForBytesWritten(10000);

QTcpSocket* NextSock = mp_Server->nextPendingConnection();
   NextSock->write("p", 1);
   NextSock->flush();
   NextSock->waitForBytesWritten(10000);

To copy to clipboard, switch view to plain text mode 

That error doesn't make sense to me as mp_Sever is created in the same thread:

void Thread2::run(void)
{
   qDebug() << "thread started:" << currentThreadId();
 
   // Create TCP interface socket
   mp_Server = new QTcpServer;
   mp_Server->listen(QHostAddress("127.0.0.1"), 7496);
 
   // Connect our socket read routine to our signal that new data has arrived
   //connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()));
   connect(mp_Server, SIGNAL(newConnection()), this, SLOT(multiplexIO()), Qt::QueuedConnection);
 
 
   // Force the first call to check our socket
   // multiplexIO();
 
   // Start thread's event loop
   exec();
   qDebug() << "thread terminated";
}

void Thread2::run(void)
{
   qDebug() << "thread started:" << currentThreadId();

   // Create TCP interface socket
   mp_Server = new QTcpServer;
   mp_Server->listen(QHostAddress("127.0.0.1"), 7496);

   // Connect our socket read routine to our signal that new data has arrived
   //connect(mp_Socket, SIGNAL(readyRead()), this, SLOT(multiplexIO()));
   connect(mp_Server, SIGNAL(newConnection()), this, SLOT(multiplexIO()), Qt::QueuedConnection);


   // Force the first call to check our socket
   // multiplexIO();

   // Start thread's event loop
   exec();
   qDebug() << "thread terminated";
}

To copy to clipboard, switch view to plain text mode 

[xenome]

Ok I think the problem is my connect() that uses "this" for the slot. Is there a way to tell the thread I want the slot to execute in the context of the newly running thread, and not the "this" parent that created the thread?

[wysota]

Did you take into consideration that readyRead() may be emitted more than once? The signal is emitted when there is something to read, not when there is everything to be read. You are probably getting two tcp segments with window size of 32kB each and processing the data fast enough to receive the readyRead() signal twice.

[xenome]

I think I figured out my problem was the connect() using a slot on the "this" object. My multiplexIO slot is actually executing in the context of the parent event loop, and not my thread. Looking at the docs it appears calling write() on my socket from the parent thread (which I guess is what I'm actually doing) could lead to "unexpected results" as my socket's QOBJECT is owned by the executing thread. For some reason I thought by connecting a signal using the "this" ptr I would ensure my slot was executing in the context of the new thread where I called connect().

The debug statement that specifies I am sending date is only seen once, so I didn't believe that was the problem.

[wysota]

I think I figured out my problem was the connect() using a slot on the "this" object. My multiplexIO slot is actually executing in the context of the parent event loop, and not my thread. Looking at the docs it appears calling write() on my socket from the parent thread (which I guess is what I'm actually doing) could lead to "unexpected results" as my socket's QOBJECT is owned by the executing thread. For some reason I thought by connecting a signal using the "this" ptr I would ensure my slot was executing in the context of the new thread where I called connect().

QThread object doesn't represent a thread. It is more like a thread controller. Only the run() method (and objects created in it) lives in the thread controlled by the QThread object. The thread will eventually stop running but the QThread object will still be there.

[xenome]

Right, I realize that now. My problem is that I'm trying to create a class that has a convenient API for sending/receiving specific data from a socket, but since the socket handling cannot happen in the thread class slots, I've got to create another object to perform the handling that is created in the run() which further complicates my interface.

[wysota]

My problem is that I'm trying to create a class that has a convenient API for sending/receiving specific data from a socket, but since the socket handling cannot happen in the thread class slots, I've got to create another object to perform the handling that is created in the run() which further complicates my interface.

You can always push the QThread object to the thread it controls using QObject::moveToThread() but actually the first question you should ask yourself is whether you need that thread in the first place. Network traffic can be handled well in the main thread so if that's the only thing your thread does, you don't need it.