Add is_alive method to CxxQtThread

[akiselev]

When combining async Rust channels and QT objects, it's really easy to end up in a situation where a message is received on the channel after the QObject has already been destroyed, causing CxxQtThread::queue to throw an exception. This PR introduces an is_alive() function to check the handle if it's been invalidated.

• Implement is_alive method in C++ CxxQtThread class
• Add is_alive to Rust Threading trait
• Generate FFI bindings for is_alive in cxx-qt-gen
• Expose is_alive as a public method on CxxQtThread in Rust
• Update documentation for Threading trait and CxxQtThread

[LeonMatthesKDAB]

Hi @akiselev , thank you for taking the time to create this PR!

overall I think the is_alive method should be useful, so it's good to have it.
However, depending on what you want to use it for, this may not do what you intend!

If you want to check is_alive before calling queue, this will not work!

e.g. this:

if thread.is_alive() {
    thread.queue(...).unwrap()
}

May panic and crash your program. The reason for this is that the CxxQtThread does not have ownership of the QObject, it is only a handle to the thread the QObject lives in.
That other thread may delete the QObject at any time, including between the calls to is_alive() and queue.

So ideally you should really prefer just using.

thread.queue(...).ok();

if you want to avoid crashing your application in that case.

If however, you want to use is_alive() to e.g. exit a loop:

while qobject_thread.is_alive() {
    // do some rust work
    // maybe message the qobject somewhere
    qobject_thread.queue(...).ok();
    // do some more rust work
}

Then it's very much a good and convenient API.

So really you can only know for certain whether a QObject has already been destroyed, not whether it's still alive, as it may be destroyed at any point.
Therefore I'd suggest renaming this to is_destructed/is_destroyed/is_dead or something like this and prominently documenting this caveat.

[akiselev]

I renamed the method to is_destroyed and added a better doc comment explaining the race condition

[LeonMatthesKDAB]

Hi @akiselev , Thank you for the update to the PR.
I'm happy to approve the new API, but suspect CI will fail because the tests haven't been updated to the new generated output.
If your system supports it, can you run the ./update_expected.sh shell script in the cxx-qt-gen directory?
That should automatically update the generation tests.

In case you have issues with the script, I can run it as well, just let me know.

[akiselev]

Updated

[ahayzen-kdab]

Looks there there is another case of a test needing updating :-)

Note you can run cargo test in the crates/cxx-qt-gen to see the ones that are failing.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f366fcb) to head (9be9362).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #984   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           71        71           
  Lines        11928     11953   +25     
=========================================
+ Hits         11928     11953   +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[LeonMatthesKDAB]

I updated the tests, so they hopefully pass CI now.

[ahayzen-kdab]

LGTM :-)

[LeonMatthesKDAB]

Great to see this finally merged.

Thank you @akiselev for your contribution!