1

I am prity new on Qt an got some issues with QSharedPointer passing around within signals. I am working with two threads (UI and a worker). The worker sends signals to the UI using signals that contain QSharedPointer of a custom QObject:

class MyObject : QObject {...}

class Window : public QWidget {
    Q_OBJECT
public slots:
    void onFound(QSharedPointer<MyObject>);
}

class Worker : public QObject {
    Q_OBJECT
public signals:
    void found(QSharedPointer<MyObject>);
}

I connect the workers found with the windows onFound with Qt::QueuedConnection because they live in different Threads and the communication therefore has to be asynchronous.

Now I observe the folowing behavior, when I pass a the last QSharedPointer refering to my object:

  • The signal moc casts the reference to my pointer to void* and arcives it.
  • The function returns resulting in the shared pointer and the corresponding object to be destroyed.

That's not what I expected - though it's reasonable. Are QSharedPointer in general designed to be passed through signals that way? And if so, is there a mecanism to keep a reference while it is queued?

I considered the folowing solutions, but I'm not totally fine with neither of them:

  1. Keep a reference somewhere that keeps reference while it is queued. But where is a reasonable place to do that and when should I let it go.
  2. Make the connection Qt::DirectConnection but then I am still have to switch the thread somehow (same situation as before)
  3. Introduce a new signal/slot with std::function parameter for passing a lambda function to be executed in the target thread and capturing a copy of my shared pointer. (This is my current solution but it's not perty elegant, isn't it?)

Do you have any other suggestions or ideas?

Myon
  • 937
  • 13
  • 23
  • I agree with @KubaOber. Qt copies the arguments from a signal to deliver them to a slot, so I'm skeptical that the pointed-to object is deleted even if the `QSharedPointer` is. How do you know the object is no longer alive? – bnaecker Mar 06 '18 at 16:10
  • If the slot doesn't get called, then why do you expect the object to survive? It won't. But your problem has nothing to do with `QSharedPointer` - and everything to do with the connection being set up incorrectly. **Show your code**. It's the same thing over and over: you have no idea what code fails, so showing little snippets with proposed solutions is a waste of your time and our time. We must see the failure first. Quite often when writing reproducers, you'll see for yourself where the problem was - since you should have code that's a screen or two long and easily comprehensible. – Kuba hasn't forgotten Monica Mar 06 '18 at 17:23

1 Answers1

1

The signal return does not destroy the corresponding object. The QMetaObject::activate call copies the shared pointer. Here's the implementation of send signal:

// SIGNAL 0
void IO::send(const QSharedPointer<Unique> & _t1)
{
    void *_a[] = { nullptr, const_cast<void*>(reinterpret_cast<const void*>(&_t1)) };
    QMetaObject::activate(this, &staticMetaObject, 0, _a);
}

You're probably experiencing a race: by the time the thread where the signal was emitted resumes execution, the destination thread has already received the object. Thus it appears in the emitting thread that the object is gone - because, by that time, it is. Yet the target object receives the instance just fine. It works fine.

The example below illustrates that it works in both single- and multi-threaded cases, and then reproduces your problem by ensuring that the race is always won by the destination thread:

// https://github.com/KubaO/stackoverflown/tree/master/questions/shared-pointer-queued-49133331
#include <QtCore>

class Unique : public QObject {
   Q_OBJECT
   int const m_id = []{
      static QAtomicInteger<int> ctr;
      return ctr.fetchAndAddOrdered(1);
   }();
public:
   int id() const { return m_id; }
};

class IO : public QObject {
   Q_OBJECT
   int m_lastId = -1;
public:
   Q_SIGNAL void send(const QSharedPointer<Unique> &);
   Q_SLOT void receive(const QSharedPointer<Unique> & u) {
      m_lastId = u->id();
   }
   int lastId() const { return m_lastId; }
};

int main(int argc, char ** argv) {
   Q_ASSERT(QT_VERSION >= QT_VERSION_CHECK(5,9,0));
   QCoreApplication app{argc, argv};
   IO src, dst;
   QObject::connect(&src, &IO::send, &dst, &IO::receive, Qt::QueuedConnection);

   QSharedPointer<Unique> u;
   QWeakPointer<Unique> alive;
   int id = -1;

   // Single-threaded case
   alive = (u.reset(new Unique), u);
   id = u->id();
   Q_ASSERT(dst.lastId() != id); // the destination hasn't seen the object yet
   emit src.send(u);
   u.reset();
   Q_ASSERT(!u);                 // we gave up ownership of the object
   Q_ASSERT(dst.lastId() != id); // the destination mustn't seen the object yet
   Q_ASSERT(alive);              // the object must be still alive
   app.processEvents();
   Q_ASSERT(dst.lastId() == id); // the destination must have seen the object now
   Q_ASSERT(!alive);             // the object should have been destroyed by now

   // Multi-threaded setup
   struct Thread : QThread { ~Thread() { quit(); wait(); } } worker;
   worker.start();
   dst.moveToThread(&worker);
   QSemaphore s_src, s_dst;

   // This thread wins the race
   alive = (u.reset(new Unique), u);
   id = u->id();
   Q_ASSERT(dst.lastId() != id);
   QTimer::singleShot(0, &dst, [&]{ s_src.release(); s_dst.acquire(); });
                                 // stop the thread
   s_src.acquire();              // wait for thread to be stopped
   emit src.send(u);
   QTimer::singleShot(0, &dst, [&]{ s_src.release(); });
                                 // resume the main thread when done
   u.reset();
   Q_ASSERT(!u);
   Q_ASSERT(alive);              // we won the race: the object must be still alive
   s_dst.release();              // get the thread running
   s_src.acquire();              // wait for the thread to be done
   Q_ASSERT(dst.lastId() == id);
   Q_ASSERT(!alive);

   // The other thread wins the race
   alive = (u.reset(new Unique), u);
   id = u->id();
   Q_ASSERT(dst.lastId() != id);
   emit src.send(u);
   QTimer::singleShot(0, &dst, [&]{ s_src.release(); });
                                // resume the main thread when done
   u.reset();
   s_src.acquire();             // wait for worker thread to be done
   Q_ASSERT(!u);
   Q_ASSERT(!alive);            // we lost the race: the object must be gone
   Q_ASSERT(dst.lastId() == id); // yet the destination has received it!

   // Ensure the rendezvous logic didn't mess up
   Q_ASSERT(id == 2);
   Q_ASSERT(!s_src.available());
   Q_ASSERT(!s_dst.available());
}

#include "main.moc"
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • Unfortunately the slot does not get called in my scenario. The only thing that worked for me was expicitly set Qt::DirectConnection - but that's not what I want. I'll double check, if register the meta-type will have an effect. – Myon Mar 06 '18 at 16:27
  • @Myon Run the test code from this answer. It's supposed to work. Go from there. You're doing something else wrong, then, quite possibly, as well. – Kuba hasn't forgotten Monica Mar 06 '18 at 17:12
  • @Myon Note the first assert in `main()`. Your reproducer should include such an assert. That's how you tell us what your Qt version is, and that's how you ensure that your code is actually executed under an expected version of Qt by anyone who wants to run it. – Kuba hasn't forgotten Monica Mar 06 '18 at 17:25
  • Thanks for your help. I don't know exactly what caused the problem but it works now. – Myon Mar 26 '18 at 12:51