-
-
Notifications
You must be signed in to change notification settings - Fork 82
Widget::destroy() should be marked unsafe #957
Comments
Starting to send the gtk PR first. |
gtk-rs/examples#296 is merged too. |
Done! |
[edit] I fully believe in what I wrote below. However, I should note (having looked around some more) that the current GTK maintainers removed gtk_widget_destroy() in GTK4. So, writing code that uses destroy() on a portion of a widget tree to break reference count cycles is not forward looking. "You can't go back home again" - you leave a project, people take it in directions you don't necessarily agree with 🤷 - but they are doing the work and you aren't. I don't fully agree with the conclusion here. (Speaking as one of the main designers of the GTK+ life cycle system as it exists now.) What does Using Consider the snippet of code: let parent = gtk::Box::new(gtk::Orientation::Vertical, 0);
win.add(&parent);
let button = gtk::Button::with_label("Click Me!");
parent.add(&button);
button.connect_clicked(clone!(@strong win, @strong parent => move |_| {
// Leaks parent button, everything they reference
win.remove(&parent);
// Would be fine, because the closure is dropped at destroy time
// unsafe { parent.destroy(); }
})); For this reason, I've always recommended that if you mean to get rid of some part of the widget tree, you should I do like that the clone! macro explicitly requires weak or strong markings - this certainly makes it less likely to leak things this way than in a language that defaults to strong reference counts. Though closures are not the only way to create strong reference cycles. (*) It's always been a little murky what you can do with a destroyed-but-still-referenced widget - some operations might work, some might no-op, some might produces Gtk-Criticals, some might segfault. But by marking |
Even in GTK4, e.g. closing a window will go through I agree that the current situation is not perfect yet because Considering this, the current situation is a compromise to put the landmines a bit further away ( |
Based on discussion on IRC
Someone wants to create a PR and also update our examples to call
Window::close()
instead?The text was updated successfully, but these errors were encountered: