c++qtqt5qt-signalsqsharedpointer

QSharedPointer gets destroyed within emit


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:

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?


Solution

  • 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"