GNOME Bugzilla – Bug 607825
[StButton] Hold ref to button until animation completes
Last modified: 2010-01-26 21:26:51 UTC
StButton has an internal animation for the border-image actor, then it connects to the "completed" signal passing itself as data. However, if the button is destroyed, nothing prevents the animation's completed signal from then causing a reference to freed memory. Holding a reference to the button is the most straightforward fix here.
Created attachment 152055 [details] [review] [StButton] Hold ref to button until animation completes
Created attachment 152056 [details] valgrind log snippet Relevant valgrind trace
Created attachment 152110 [details] backtrace Not sure if this is related but it seems that this is not the only crasher (was running with this patch after Colin suggested it on IRC). Backtrace is attached; if you want me to file a new bug just ask.
Comment on attachment 152055 [details] [review] [StButton] Hold ref to button until animation completes I am a tiny bit worried that this could leak the button if, for some reason, "completed" was not emitted. Another possibility would be to do anim = clutter_actor_get_animation (CLUTTER_ACTOR (gobject)); if (anim) clutter_animation_completed (anim); from st_button_dispose(). However, I'm not especially familiar with ClutterAnimation, and maybe there's really nothing to worry about here.
Created attachment 152332 [details] [review] [StButton] Hold ref to self until animation completes StButton has an internal animation for the border-image actor, then it connects to the "completed" signal passing itself as data. However, if the button is destroyed, nothing prevents the animation's completed signal from then causing a reference to freed memory. Holding a reference to the button is the most straightforward fix here.
Comment on attachment 152332 [details] [review] [StButton] Hold ref to self until animation completes meh. that unref in st_button_dispose_old_bg seems pretty magical. i like the first version better. your call. hm... and does this do the right thing if the style changes, and then changes again while the first animation is still in progress?
(In reply to comment #6) > (From update of attachment 152332 [details] [review]) > meh. that unref in st_button_dispose_old_bg seems pretty magical. i like the > first version better. your call. I'm more confident in this one. > hm... and does this do the right thing if the style changes, and then changes > again while the first animation is still in progress? I believe so, because we call st_button_dispose_old_bg as the first thing in style_changed, which will unref if we had an animation in progress.
Attachment 152332 [details] pushed as d7e0051 - [StButton] Hold ref to self until animation completes