Problem with SIGNAL/SLOT lifecycle

DRoscoe

NOTE: This particular example is for my iOS app, but the question itself is more general, namely the lifecycle of SIGNAL/SLOT connections.

I am stress testing my iOS application by putting it to sleep and waking it up repeatedly. This works fine for a random number of times and then I start running into problems with my socket connections. The problem I am outlining below happened to occur after a single sleep/wake cycle of the iPad. When the app is first launched, I am logging the following output, which I will explain further below:

INFO: ::run()
        - initial_start: true
        - clean_start: false
        - shutting_down: false
INFO: [2000/01/01 00:00:00.000] TAP Display Version: 15-3.8.2
INFO: ::createCommunicationsThreads()
INFO: ::getHostAddress()
INFO: Host name: 159.62.163.25
INFO: BEGIN - TAP Service connecting to host 159.62.163.25
INFO: END - TAP Service connecting to host 159.62.163.25
INFO: ::handleServiceConnectionEstablished()
INFO: [2000/01/01 00:00:00.000] Sending Go Command
INFO: [2000/01/01 00:00:00.000] Go Succeeded: Connecting to Display Adapter...
INFO: BEGIN - attempt TAP DISPLAY ADAPTER connection
INFO: END - attempt TAP DISPLAY ADAPTER connection
INFO: ::signalConnectionEstablished()

The communication threads and worker objects are created in the call to ::createCommunicationsThreads():

bool TapDisplaySingleton::createCommunicationsThreads()
{
  engine_comm_thread = new QThread(this);
  service_comm_thread = new QThread(this);
  
  QHostAddress host_address = getHostAddress();

  eng_comm_worker = new TcpCommunicationHandler(host_address,
                                                    engine_port,
                                                    "TAP DISPLAY ADAPTER");
  // move the worker object to the comm handler thread
  eng_comm_worker->moveToThread(engine_comm_thread);

  // create service communication handler
  quint16 svc_port = 49997;
  svc_comm_worker = new TapServiceCommunicationHandler(host_address,
                                                           svc_port,
                                                           "TAP SERVICE");
  // move the worker object to the comm handler thread
  svc_comm_worker->moveToThread(service_comm_thread);

  // Set up all communication signal handlers
  setCommunicationSignals();

  if (eng_comm_worker)
  {
    engine_comm_thread->start();
  }

  if (svc_comm_worker)
  {
    service_comm_thread->start();
  }

  return true;
}

In the call to ::setCommunicationSignals() I am setting up several different signals to/from the comm_worker objects, including the following:

QObject::connect(svc_comm_worker, SIGNAL(connectionEstablished()),
                      this, SLOT(handleServiceConnectionEstablished()));
QObject::connect(eng_comm_worker, SIGNAL(connectionEstablished()),
                     this, SLOT(handleEngineConnectionEstablished()));

// lifecycle control
    QObject::connect(this, SIGNAL(killEngineCommThread()),
                     eng_comm_worker, SLOT(killThread()));
    QObject::connect(eng_comm_worker, SIGNAL(finished()),
                     engine_comm_thread, SLOT(quit());
    QObject::connect(eng_comm_worker, SIGNAL(finished()),
                     eng_comm_worker, SLOT(deleteLater()));
    QObject::connect(engine_comm_thread, SIGNAL(finished()), 
                     engine_comm_thread, SLOT(deleteLater()));

Note here that 'this' is the main app class. It is the parent of both thread objects. The comm_worker objects have to be parentless (which I think is part of the problem outlined below).

When the app initially starts or wakes up from hibernation, a signal is emitted to initiate the "service" connection:

emit connectToService();

When the service connection is made, the handleServiceConnectionEstablished() slot is invoked which then sends a "GO" command to the service host. The purpose of this is to bootstrap the rest of the system. If this process is successful, a signal is emitted to initiate the next step of the process, which is to connect to the TAP Display Adapter:

void TapDisplaySingleton::handleGoCommandSuccessful()
{
  logInfo("Go Succeeded: Connecting to Display Adapter...");
  emit connectToDisplayAdapter();
}

When the connection to the Display Adapter is made, the program then enters is OPERATE mode.

I put the device to sleep. As indicated above the destruction of the comm_worker objects is tied to the life cycle of the thread it lives on. It appears to be working as intended since I am seeing this upon sleep:

Received application state change:  2
INFO: [2013/11/06 21:01:21.000] Suspending Operations at Fri Nov 18 19:53:16 2016 GMT
INFO: ::killThread()
INFO: ::~TapDisplayCommunicationHandler()
INFO: ::killThread()
INFO: ~TapServiceCommunicationHandler()

The Qt documentation states:
"A signal-slot connection is removed when either of the objects involved are destroyed."

I assumed that killing the comm_worker objects would also kill the connections set on it. That does not appear to be the case.

I wake the device up:

Received application state change:  4
INFO: ::reactivateTapDisplay()
        - clean_start: false
INFO: ::createCommunicationsThreads()
INFO: ::getHostAddress()
INFO: Host name: 159.62.163.25
INFO: Connecting to TAP Service...
INFO: BEGIN - TAP Service connecting to host 159.62.163.25
INFO: [2013/11/06 21:01:23.000] Reactivating Operations at Fri Nov 18 19:53:22 2016 GMT
INFO: END - TAP Service connecting to host 159.62.163.25
QIODevice::write: device not open
QIODevice::write: device not open
INFO: ::handleServiceConnectionEstablished()
INFO: [2013/11/06 21:01:23.000] Sending Go Command
INFO: [2013/11/06 21:01:23.000] Go Succeeded: Connecting to Display Adapter...
INFO: [2013/11/06 21:01:23.000] Go Succeeded: Connecting to Display Adapter...
INFO: BEGIN - attempt TAP DISPLAY ADAPTER connection
INFO: END - attempt TAP DISPLAY ADAPTER connection
INFO: BEGIN - attempt TAP DISPLAY ADAPTER connection
QAbstractSocket::connectToHost() called when already looking up or connecting/connected to "159.62.163.25"
INFO: TcpCommunicationHandler (TAP DISPLAY ADAPTER) received an error:
         - 19
INFO: END - attempt TAP DISPLAY ADAPTER connection

As in the initial startup, I created new QThreads and comm_worker objects and set up new connections to them. I am emitting a single "GO" command, but receiving two responses!

INFO: [2013/11/06 21:01:23.000] Sending Go Command
INFO: [2013/11/06 21:01:23.000] Go Succeeded: Connecting to Display Adapter...
INFO: [2013/11/06 21:01:23.000] Go Succeeded: Connecting to Display Adapter...

And there's no doubt the signal is real, because the code is attempting to establish a connection to the Display Adapter twice and the socket code is complaining about it.

INFO: BEGIN - attempt TAP DISPLAY ADAPTER connection
INFO: END - attempt TAP DISPLAY ADAPTER connection
INFO: BEGIN - attempt TAP DISPLAY ADAPTER connection
QAbstractSocket::connectToHost() called when already looking up or connecting/connected to "159.62.163.25"
INFO: TcpCommunicationHandler (TAP DISPLAY ADAPTER) received an error:
         - 19
INFO: END - attempt TAP DISPLAY ADAPTER connection

I have since confirmed that the number of responses and connection attempts matches the number of times I sleep/wake the iPad before experiencing a problem. For example, in one case I slept/woke the iPad 8 times before running into a problem. There were exactly 8 responses to the "GO" command and exactly 8 attempts to make a Display Adapter connection.

There is also no event loop running on the orphaned comm_worker objects, so these bogus signal/slot calls are being made on the main class's event loop (i.e. direct method calls).

Why weren't the connections to the killed objects also killed as indicated in the documentation? I suspect it has to do with the fact that the comm_worker objects are parentless, so the main class does not know the objects are destroyed? This seems odd though because that would require every object to have some parent/sibling link to any object it communicates with.

The memory was destroyed, so how was it able to execute code on the freed object without crashing?

VRonin

We are missing a few details... what does TcpCommunicationHandler::killThread do?
could also post the code of the worker where it sends the finished signal?

DRoscoe

@VRonin Here is the code you requested:

void TcpCommunicationHandler::killThread()
{
  qDebug() << "::killThread()";
  
  socket->disconnectFromHost();
  emit finished();
}

What is really baffling me is I only see this behavior when I start having connection problems. For example, I can wake/sleep many times and there's no problem. All of the signals and slots work as expected. When it finally does hiccup, its as if it complaining about every previous connection.

DRoscoe

I've solved the problem. It turns out that there was nothing wrong with the signal/slot lifecycle. There was, however, something wrong with my assumption about how signal/slot connections are managed by Qt.

As I mentioned previously, when the system first starts, I connect to the "service" and send a "GO" command:

void TapDisplaySingleton::handleServiceConnectionEstablished()
{
  if (initial_start || clean_start)
  {
    sendGoCommand();
  }
  [...]

When the "GO" command is sent, I emit another signal, goCommandSuccessful() which is emitted from my main class thread, to my main class thread. It's a placeholder for when the "service" can be modified to tell us when it's spawned the rest of the system. This turns out to be the problem because the signal/slot connection is set up in the ::setCommunicationSignals() method:

    QObject::connect(this, SIGNAL(goCommandSuccessful()),
                      this, SLOT(handleGoCommandSuccessful()));

It was my assumption that since nothing about this connection ever changes, Qt would register it the first time, and ignore it all the other times it's called. Turns out this is not the case. Qt allowed me to define multiple listeners of this signal even though the listener was the same as the previous connection.

So, the next time we have to send a "GO" command (clean restart) such as when an error occurs upon waking and the app needs to reinitialize itself, the emission of goCommandSuccessful() will now be received by my main class object the exact number of times that I've called ::setCommunicationSignals()

That's what happens when you (I) make assumptions!

Thanks!
-Dave

hskoglund

Hi, you can avoid that particular pitfall by appending Qt::UniqueConnection, like this:

Object::connect(this, SIGNAL(goCommandSuccessful()),
                      this, SLOT(handleGoCommandSuccessful()),Qt::UniqueConnection);

JKSH

@DRoscoe said in Problem with SIGNAL/SLOT lifecycle:

Qt allowed me to define multiple listeners of this signal even though the listener was the same as the previous connection.

Yep, this is documented at http://doc.qt.io/qt-5/signalsandslots.html :

"By default, for every connection you make, a signal is emitted; two signals are emitted for duplicate connections. You can break all of these connections with a single disconnect() call. If you pass the Qt::UniqueConnection type, the connection will only be made if it is not a duplicate. If there is already a duplicate (exact same signal to the exact same slot on the same objects), the connection will fail and connect will return false"

DRoscoe

@JKSH That's good to know. In this case, however, that connection should never have been made there. Its not a communications signal and it only needs to be set once. I can do it in the constructor of my main app class