Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Threading on all (even imported) QObjects #1155

Open
LeonMatthesKDAB opened this issue Jan 13, 2025 · 0 comments
Open

Support Threading on all (even imported) QObjects #1155

LeonMatthesKDAB opened this issue Jan 13, 2025 · 0 comments
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 👷 refactor Something needs to change

Comments

@LeonMatthesKDAB
Copy link
Collaborator

LeonMatthesKDAB commented Jan 13, 2025

In a recent discussion with @milianw , he come up with an interesting idea to extend support for cxx_qt::Threading to all QObjects, even imported ones.

Current Implementation

Currently we use CxxQtGuardedPointer as a thread-safe version of QPointer, which however requires the QObject to be aware of the GuardedPointer, so it only works with our own QObjects.

New Idea

We could get around this requirement by creating a "context" object, which is manually memory-managed and is basically only responsible for keeping a handle to the thread on which the original QObject lives.

Example by @milianw :

    // no parent! manual lifetime management - make sure to delete once QtThread goes out of scope!
    auto context = new QObject;
    // only safely accessible from the mainthread
    auto safeObject = QPointer(object);

    QThread::create([safeObject, context]() {
        /// Use invokeMethod on the context instead of on the object
        QMetaObject::invokeMethod(context, [safeObject, context]() {
            if (safeObject) {
                // actually "invoke" the slot here, directly
                safeObject->dataReady(...);
            }
            context->deleteLater();
        });
    });

Note: The memory-management of context will look different in our case, as we either need to create a context object per CxxQtThread, or per queue call.
Note that if we delete the context object in the destructor of CxxQtThread this would potentially delete it too early, as the invokeMethod may not yet have finished.

Implications

Note however, that this expects the context and the original QObject to remain on the same thread!
Which means either the threading itself, or moveToThread must be unsafe.

Currently QObjects don't implement Send, which is a bit at odds with moveToThread, which does indeed allow moving QObjects between threads.
So it's likely that moveToThread would have to be unsafe either way, which would be one way to remedy this conundrum.
If it's unsafe to call moveToThread, we can ask callers to guarantee that no QtThread instance for this QObject currently exists.

moveToThread also emits a QEvent::ThreadChange, which we could catch via an event filter.
https://doc.qt.io/qt-6/qobject.html#moveToThread
So another option would be to change the contexts thread at the same time.
However, the thread safety implications of this are also questionable.

@LeonMatthesKDAB LeonMatthesKDAB added ⬆️ feature New feature or request 🤔 discussion Feedback welcome 👷 refactor Something needs to change labels Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 👷 refactor Something needs to change
Projects
Status: Todo
Development

No branches or pull requests

1 participant