Fix const detection with two ways binding

The const detection for two way binding was not detecting change if one
of the property was set to a const value in a component using it.

This would cause the compiler to generate call set_content on one of the
property in a two way bindings, and later, the "const sentinel" be
present in the dependency list, causing crash.

To avoid segfault for similar bug in the future, added added an assert!
in the property system to detect that.

Fixes #2185
This commit is contained in:
Olivier Goffart 2023-02-06 17:46:27 +01:00 committed by Olivier Goffart
parent 7990bfe6f0
commit dddfcf572d
3 changed files with 81 additions and 5 deletions

View file

@ -524,8 +524,8 @@ fn propagate_is_set_on_aliases(component: &Rc<Component>, reverse_aliases: &mut
}
fn mark_alias(alias: &NamedReference) {
alias.mark_as_set();
if !alias.is_externally_modified() {
alias.mark_as_set();
if let Some(bind) = alias.element().borrow().bindings.get(alias.name()) {
propagate_alias(&bind.borrow())
}

View file

@ -493,8 +493,8 @@ impl PropertyHandle {
(*binding).dependencies.set(0);
} else {
DependencyListHead::mem_move(
(&mut (*binding).dependencies) as *mut _ as *mut _,
self.handle.as_ptr() as *mut _,
(*binding).dependencies.as_ptr() as *mut DependencyListHead,
self.handle.as_ptr() as *mut DependencyListHead,
);
}
((*binding).vtable.drop)(binding);
@ -540,8 +540,8 @@ impl PropertyHandle {
(*binding).dependencies.set(const_sentinel);
} else {
DependencyListHead::mem_move(
self.handle.as_ptr() as *mut _,
(&mut (*binding).dependencies) as *mut _ as *mut _,
self.handle.as_ptr() as *mut DependencyListHead,
(*binding).dependencies.as_ptr() as *mut DependencyListHead,
);
}
}
@ -657,10 +657,23 @@ impl Drop for PropertyHandle {
/// Safety: the dependency list must be valid and consistent
unsafe fn mark_dependencies_dirty(dependencies: *mut DependencyListHead) {
debug_assert!(!core::ptr::eq(
*(dependencies as *mut *const u32),
(&CONSTANT_PROPERTY_SENTINEL) as *const u32,
));
DependencyListHead::for_each(&*dependencies, |binding| {
let binding: &BindingHolder = &**binding;
let was_dirty = binding.dirty.replace(true);
(binding.vtable.mark_dirty)(binding as *const BindingHolder, was_dirty);
assert!(
!core::ptr::eq(
binding.dependencies.as_ptr() as *const u32,
(&CONSTANT_PROPERTY_SENTINEL) as *const u32,
),
"Const property marked as dirty"
);
mark_dependencies_dirty(binding.dependencies.as_ptr() as *mut DependencyListHead)
});
}
@ -1247,6 +1260,10 @@ impl<DirtyHandler: PropertyDirtyHandler> PropertyTracker<DirtyHandler> {
if CURRENT_BINDING.is_set() {
CURRENT_BINDING.with(|cur_binding| {
if let Some(cur_binding) = cur_binding {
debug_assert!(!core::ptr::eq(
self.holder.dependencies.get() as *const u32,
(&CONSTANT_PROPERTY_SENTINEL) as *const u32,
));
cur_binding.register_self_as_dependency(
self.holder.dependencies.as_ptr() as *mut DependencyListHead,
#[cfg(slint_debug_property)]

View file

@ -0,0 +1,59 @@
// Copyright © SixtyFPS GmbH <info@slint-ui.com>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial
component Buggy {
in-out property<string> magic: "Nope";
property<bool> condition: false;
public function change_condition() {
condition = !condition;
}
HorizontalLayout {
if root.condition: TouchArea {
property <string> innermagic <=> root.magic;
clicked => { self.innermagic = "Hello"; }
horizontal-stretch: 1;
}
Rectangle {
background: red;
TouchArea {
clicked => { change-condition() }
}
}
}
}
component TestCase {
width: 300px;
height: 100px;
public function change_condition() {
b.change-condition();
}
b := Buggy {
magic: "Hi";
}
out property <string> magic: b.magic;
}
/*
```rust
let instance = TestCase::new();
assert_eq!(instance.get_magic(), slint::SharedString::from("Hi"));
instance.invoke_change_condition();
instance.invoke_change_condition();
instance.invoke_change_condition();
assert_eq!(instance.get_magic(), slint::SharedString::from("Hi"));
slint_testing::send_mouse_click(&instance, 10., 10.);
assert_eq!(instance.get_magic(), slint::SharedString::from("Hello"));
```
*/