woboq / qmetaobject-rs

Integrate Qml and Rust by building the QMetaObject at compile time.
MIT License
647 stars 89 forks source link

Single mutable reference rules easily invalidated with async code #151

Open sphaerophoria opened 3 years ago

sphaerophoria commented 3 years ago

Dispatching a future using a mutable reference to an object accessible from QML results in multiple mutable references.

Here is an example that models a use case I've been using.

use qmetaobject::prelude::*;
use cstr::cstr;

#[derive(QObject, Default)]
struct MyAsyncObject {
    base: qt_base_class!(trait QObject),
    some_other_fn: qt_method!(fn some_other_fn(&mut self) {
        println!("Hello with mutable self");
    }),
}

impl MyAsyncObject {
    async fn long_function(&mut self) {
        println!("Long function with mutable self");
        futures::future::pending::<()>().await;
        println!("Finished long function with mutable self");
    }
}

fn main() {
    // Create a QML engine from rust
    let mut engine = QmlEngine::new();
    let my_async_object = QObjectBox::new(MyAsyncObject::default());
    let pinned = my_async_object.pinned();

    engine.set_object_property("async".into(), pinned);

    qmetaobject::future::execute_async(async move {
        let pinned = my_async_object.pinned();
        pinned.borrow_mut().long_function().await;
    });

    // (Here the QML code is inline, but one can also load from a file)
    engine.load_data(r#"
        import QtQuick 2.6
        import QtQuick.Window 2.0
        import QtQuick.Controls 2.15
        import QtQuick.Layouts 1.15

        Window {
            visible: true
            RowLayout {
                Button {
                    text: "Other function"
                    onClicked: async.some_other_fn()
                }
            }
        }
    "#.into());
    engine.exec();
}

Pressing the button in the UI shows the following text

Long function with mutable self
Hello with mutable self
Hello with mutable self
Hello with mutable self
Hello with mutable self

To prove this isn't a contrived example, imagine an application that has gui elements driven both by user interaction and code running on a backend. Events from the backend have to be propagated to the gui. This could be done by giving the QT event loop an async call that polls a tokio socket for events.

sphaerophoria commented 3 years ago

I believe this is due to this fixme. As far as I can tell refcell usages are propagated to C++ and read back, so this isn't an impossible case to guard against, but it's just avoided because of how QML will recurse into our struct again when handling signals.

I don't have a good answer for a fix here, maybe there's a way to use the type system to prevent users from putting async functions requiring QObjects onto the exeuctor. Maybe async functions should just not be supported on QObjects at all.

I believe this shouldn't happen using the threading model shown in the docs. In this case the event loop is blocked until the queued operation is complete. Maybe a warning in the docs around QObjectPinned::borrow_mut and async code is a good enough fix for now

obj-obj commented 1 year ago

Yeah, I'm trying to make an app and ended up doing exactly this, but the backend code driving some of the UI just freezes 6-8 seconds after starting the app

ogoffart commented 1 year ago

yeah, the api is actually unsound :-( https://github.com/woboq/qmetaobject-rs/issues/11