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

Find a solution to make sure there is no several &mut reference to a QObject or QGadget #11

Open
ogoffart opened this issue Sep 18, 2018 · 1 comment
Assignees
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness

Comments

@ogoffart
Copy link
Member

Currently, when one use #[derive(QObject)] struct RustName { ... } it is possible to get several mutable reference to the same object at the same time.
Also we must ensure that mutable references are not moved.

For any QObject, there is the CPP object, and the Rust object. each must contains a pointer to the other. Currently, the CPP object contains a *mut BaseTrait TraitObject pointer, where BaseTrait is the trait in the qt_base_class!(). But then this is transfomred in a reference &mut StructName which is then passed a a &mut self to the setter or qt_method. Or within virtual functions.
There is also the QPointer class which allow a cast to &StructName

I was thinking that instead of putting the StructName in a Box<StructName>, I would instead use Box<RefCell<StructName>> and store a *const RefCell<BaseTrait> in the CPP object. (I would still need to store a *const QObject for Drop and to get the actual pointer`) This way, one can safely access this.

There is still the problem of the feature that currently, RustName don't necessarily need to be on the heap. We could change that.
But there is still the current feature of property whose type derives from QObject, these are not on the heap, not in a refcell. Maybe we need to force the property in a refcell then.

As to force the location: I am makng a wrapper around &RefCell<T> where T : QObject + ?Sized And the borrow_mut returns:

struct QObjectRefMut<'b, T : QObject + ?Sized + 'b> {
    old_value : *mut c_void,  // Store the old value so we can check it is still valid when it goes out of scope
    inner: std::cell::RefMut<'b, T>
}
impl<'b, T: QObject + ?Sized> std::ops::Deref for QObjectRefMut<'b, T> { ... }
impl<'b, T: QObject + ?Sized> std::ops::DerefMut for QObjectRefMut<'b, T> { ... }
impl<'b, T : QObject + ?Sized + 'b> Drop for QObjectRefMut<'b, T> {
    fn drop(&mut self) {
        assert_eq!(self.old_value, self.get_cpp_object(), "Internal pointer changed while borrowed");
    }
}

So this make sure nobody moves the RustName away.

I have some work in progress, let's see if it is the best approach.

@ogoffart ogoffart self-assigned this Sep 18, 2018
@ogoffart
Copy link
Member Author

ogoffart commented Sep 28, 2018

Now the rust object is always held in a RefCell, however, the code does not borrow_mut when the call is coming from Qt (property access / method access)

@ogoffart ogoffart added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
Projects
None yet
Development

No branches or pull requests

1 participant