slint icon indicating copy to clipboard operation
slint copied to clipboard

Crash when accessing two way binding having binding referencing destroyed component

Open ogoffart opened this issue 1 year ago • 0 comments

Testcase:

Sub := HorizontalLayout {
    property <string> val: ext;
    property <string> ext;
}

App := Window {
    property <string> value;
    property <bool> cond: true;
    property <string> external: "HELLO";
    if cond: Sub {
        val <=> value;
        ext: external;
    }
    Text { text: value; }
    TouchArea { clicked=> { cond = !cond; external += "PLOP";} }
}

When clicking, this panic (unwrap()) when compiled with rust generated code, or crash in the interpreter because of our use of unsafe.

The bug is that we create a two way binding, and we assign a binding (self.ext) to the two way binding which links value and val But then we destroy the Sub component but the binding is still there on the two way linked thing.

So sumary in pseudo code:

link_two_way(&root.val, &sub.value);
  // -> sets a two way binding to both property, and there is a shared third property with the actual value
sub.value.set_binding({ let sub_weak = sub.downgrade(); move || sub_weak.unwrap().ext.get() );
  // -> sets the binding to the shared property instead of sub.value
//...
drop(sub);  // when the cond becomes false
root.val.get(); // uhuh, will run the binding of the shared property which access deleted sub

(note that there is also a memory leak there because when we delete sub, the shared property is only shared with one other property. And then when we create sub again we will call another call to link_two_way which will create another shared property. So in the end we have a potentially infinite linked list of Rc<Property<T>> that are actually not shared. But that's a different problem that could be fixed by not re-creating another Rc.)

(another issue is that this is only present with inlining. If we use a Rectangle instead of a HorizontalLayout, then the crash can't be reproduced because then, set_binding will be called before link_two_way, and link_two_way will erase the binding.)

ogoffart avatar Jul 07 '22 09:07 ogoffart