Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Fix an ownership issue #932

Merged
merged 1 commit into from
Jan 3, 2020
Merged

Conversation

hfiguiere
Copy link
Contributor

Without this I get these valgrind errors (or a crash without valgrind, that cause also gdb to crash)

==1466829== Invalid read of size 8
==1466829==    at 0x52B0189: gtk_widget_dispatch_child_properties_changed (gtkwidget.c:4491)
==1466829==    by 0x1263B9: <T as gtk::subclass::widget::WidgetImplExt>::parent_dispatch_child_properties_changed ()
==1466829==    by 0x123091: gtk::subclass::widget::WidgetImpl::dispatch_child_properties_changed ()
==1466829==    by 0x131A4A: gtk::subclass::widget::widget_dispatch_child_properties_changed ()
==1466829==    by 0x52B28FF: g_object_notify_queue_thaw (gobjectnotifyqueue.c:136)
==1466829==    by 0x52B28FF: gtk_widget_thaw_child_notify (gtkwidget.c:4565)
==1466829==    by 0x5051042: gtk_box_pack (gtkbox.c:1561)
==1466829==    by 0x118673: <O as gtk::auto::box_::BoxExt>::pack_start (box_.rs:539)
==1466829==    by 0x118B5B: widget_test::main ()
==1466829==    by 0x11885F: std::rt::lang_start::{{closure}} ()
==1466829==    by 0x164312: std::panicking::try::do_call (in /home/hub/source/niepce/target/debug/examples/widget-test)
==1466829==    by 0x165B19: __rust_maybe_catch_panic (in /home/hub/source/niepce/target/debug/examples/widget-test)
==1466829==    by 0x164D8C: std::rt::lang_start_internal (in /home/hub/source/niepce/target/debug/examples/widget-test)
==1466829==  Address 0x17c119a0 is 0 bytes inside a block of size 16 free'd
==1466829==    at 0x483AA0C: free (vg_replace_malloc.c:540)
==1466829==    by 0x14B572: alloc::alloc::dealloc ()
==1466829==    by 0x14B3F9: <alloc::alloc::Global as core::alloc::Alloc>::dealloc ()
==1466829==    by 0x143638: alloc::raw_vec::RawVec<T,A>::dealloc_buffer (raw_vec.rs:742)
==1466829==    by 0x146FEE: <alloc::raw_vec::RawVec<T,A> as core::ops::drop::Drop>::drop (raw_vec.rs:751)
==1466829==    by 0x12E96E: core::ptr::real_drop_in_place ()
==1466829==    by 0x12F9D1: core::ptr::real_drop_in_place ()
==1466829==    by 0x126335: <T as gtk::subclass::widget::WidgetImplExt>::parent_dispatch_child_properties_changed ()
==1466829==    by 0x123091: gtk::subclass::widget::WidgetImpl::dispatch_child_properties_changed ()
==1466829==    by 0x131A4A: gtk::subclass::widget::widget_dispatch_child_properties_changed ()
==1466829==    by 0x52B28FF: g_object_notify_queue_thaw (gobjectnotifyqueue.c:136)
==1466829==    by 0x52B28FF: gtk_widget_thaw_child_notify (gtkwidget.c:4565)
==1466829==    by 0x5051042: gtk_box_pack (gtkbox.c:1561)
==1466829==  Block was alloc'd at
==1466829==    at 0x483980B: malloc (vg_replace_malloc.c:309)
==1466829==    by 0x14B50B: alloc::alloc::alloc ()
==1466829==    by 0x14B391: <alloc::alloc::Global as core::alloc::Alloc>::alloc ()
==1466829==    by 0x145107: alloc::raw_vec::RawVec<T,A>::reserve_internal (raw_vec.rs:695)
==1466829==    by 0x146D19: alloc::raw_vec::RawVec<T,A>::reserve (raw_vec.rs:520)
==1466829==    by 0x140DE9: alloc::vec::Vec<T>::reserve ()
==1466829==    by 0x12AB08: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::spec_extend ()
==1466829==    by 0x12AD10: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter ()
==1466829==    by 0x12AE0C: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter ()
==1466829==    by 0x12FAA4: core::iter::traits::iterator::Iterator::collect ()
==1466829==    by 0x126302: <T as gtk::subclass::widget::WidgetImplExt>::parent_dispatch_child_properties_changed ()
==1466829==    by 0x123091: gtk::subclass::widget::WidgetImpl::dispatch_child_properties_changed ()
==1466829== 
==1466829== 

@GuillaumeGomez
Copy link
Member

Great catch, thanks a lot!

I guess we'll need a minor release...

cc @EPashkin @sdroege

.collect::<Vec<_>>()
.as_mut_ptr();
.collect::<Vec<_>>();
let pspecs_ptr = pspecs_array.as_mut_ptr();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO .map(|p| p.to_glib_none().0) leak too
and better use something like this

let stashes = ... .map(|p| p.to_glib_none()).collect...();
let pspecs_ptr = stashes.0.as_mut_ptr();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW valgrind doesn't show this leaking.

@sdroege
Copy link
Member

sdroege commented Jan 3, 2020 via email

@sdroege
Copy link
Member

sdroege commented Jan 3, 2020

@GuillaumeGomez Good to go and a bugfix release indeed, please :)

@EPashkin
Copy link
Member

EPashkin commented Jan 3, 2020

👍

@GuillaumeGomez GuillaumeGomez merged commit f3201e9 into gtk-rs:master Jan 3, 2020
@hfiguiere hfiguiere deleted the pspecs-crash branch January 13, 2020 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants