From cbf5f3d622e7956bcbf551231d8e8029ac5f004d Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sun, 8 Jan 2017 12:29:30 +0100 Subject: Fix crash when changing Pane widgets -- #7198 Gtkmm2Ext::Pane::on_add() uses a pointer to a std::vector<> element in the destroy notify callback. If the vector is modified, that pointer becomes invalid. Add 2 widgets "A", "B". remove "B", add another one "C". Now if A is destroyed, notify_child_destroyed(PTR) points to invalid memory and not to "A". --- libs/gtkmm2ext/gtkmm2ext/pane.h | 2 +- libs/gtkmm2ext/pane.cc | 92 ++++++++++++++++++++++------------------- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/libs/gtkmm2ext/gtkmm2ext/pane.h b/libs/gtkmm2ext/gtkmm2ext/pane.h index 21fba2e9b1..d8367b36f6 100644 --- a/libs/gtkmm2ext/gtkmm2ext/pane.h +++ b/libs/gtkmm2ext/gtkmm2ext/pane.h @@ -55,7 +55,7 @@ class LIBGTKMM2EXT_API Pane : public Gtk::Container Child (Pane* p, Gtk::Widget* widget, uint32_t ms) : pane (p), w (widget), minsize (ms) {} }; - typedef std::vector Children; + typedef std::vector Children; Pane (bool horizontal); ~Pane(); diff --git a/libs/gtkmm2ext/pane.cc b/libs/gtkmm2ext/pane.cc index 8e8b337f32..38a5115358 100644 --- a/libs/gtkmm2ext/pane.cc +++ b/libs/gtkmm2ext/pane.cc @@ -49,19 +49,21 @@ Pane::Pane (bool h) Pane::~Pane () { for (Children::iterator c = children.begin(); c != children.end(); ++c) { - c->show_con.disconnect (); - c->hide_con.disconnect (); - c->w->remove_destroy_notify_callback (&(*c)); - c->w->unparent (); + (*c)->show_con.disconnect (); + (*c)->hide_con.disconnect (); + (*c)->w->remove_destroy_notify_callback (*c); + (*c)->w->unparent (); + delete (*c); } + children.clear (); } void Pane::set_child_minsize (Gtk::Widget const& w, int32_t minsize) { for (Children::iterator c = children.begin(); c != children.end(); ++c) { - if (c->w == &w) { - c->minsize = minsize; + if ((*c)->w == &w) { + (*c)->minsize = minsize; break; } } @@ -95,26 +97,26 @@ Pane::on_size_request (GtkRequisition* req) largest.width = 0; } - for (Children::iterator child = children.begin(); child != children.end(); ++child) { + for (Children::iterator c = children.begin(); c != children.end(); ++c) { GtkRequisition r; - if (!child->w->is_visible ()) { + if (!(*c)->w->is_visible ()) { continue; } - child->w->size_request (r); + (*c)->w->size_request (r); if (horizontal) { largest.height = max (largest.height, r.height); - if (child->minsize) { - largest.width += child->minsize; + if ((*c)->minsize) { + largest.width += (*c)->minsize; } else { largest.width += r.width; } } else { largest.width = max (largest.width, r.width); - if (child->minsize) { - largest.height += child->minsize; + if ((*c)->minsize) { + largest.height += (*c)->minsize; } else { largest.height += r.height; } @@ -156,18 +158,18 @@ Pane::handle_child_visibility () void Pane::on_add (Widget* w) { - children.push_back (Child (this, w, 0)); - Child& kid = children.back (); + children.push_back (new Child (this, w, 0)); + Child* kid = children.back (); w->set_parent (*this); /* Gtkmm 2.4 does not correctly arrange for ::on_remove() to be called for custom containers that derive from Gtk::Container. So ... we need to ensure that we hear about child destruction ourselves. */ - w->add_destroy_notify_callback (&children.back(), &Pane::notify_child_destroyed); + w->add_destroy_notify_callback (kid, &Pane::notify_child_destroyed); - kid.show_con = w->signal_show().connect (sigc::mem_fun (*this, &Pane::handle_child_visibility)); - kid.hide_con = w->signal_hide().connect (sigc::mem_fun (*this, &Pane::handle_child_visibility)); + kid->show_con = w->signal_show().connect (sigc::mem_fun (*this, &Pane::handle_child_visibility)); + kid->hide_con = w->signal_hide().connect (sigc::mem_fun (*this, &Pane::handle_child_visibility)); while (dividers.size() < (children.size() - 1)) { add_divider (); @@ -185,10 +187,12 @@ void* Pane::child_destroyed (Gtk::Widget* w) { for (Children::iterator c = children.begin(); c != children.end(); ++c) { - if (c->w == w) { - c->show_con.disconnect (); - c->hide_con.disconnect (); + if ((*c)->w == w) { + (*c)->show_con.disconnect (); + (*c)->hide_con.disconnect (); + Child* kid = *c; children.erase (c); + delete kid; break; } } @@ -199,12 +203,14 @@ void Pane::on_remove (Widget* w) { for (Children::iterator c = children.begin(); c != children.end(); ++c) { - if (c->w == w) { - c->show_con.disconnect (); - c->hide_con.disconnect (); - w->remove_destroy_notify_callback (&(*c)); + if ((*c)->w == w) { + (*c)->show_con.disconnect (); + (*c)->hide_con.disconnect (); + w->remove_destroy_notify_callback (*c); w->unparent (); + Child* kid = *c; children.erase (c); + delete kid; break; } } @@ -240,7 +246,7 @@ Pane::reallocate (Gtk::Allocation const & alloc) if (children.size() == 1) { /* only child gets the full allocation */ - children.front().w->size_allocate (alloc); + children.front()->w->size_allocate (alloc); return; } @@ -259,7 +265,7 @@ Pane::reallocate (Gtk::Allocation const & alloc) /* skip initial hidden children */ while (child != children.end()) { - if (child->w->is_visible()) { + if ((*child)->w->is_visible()) { break; } ++child; @@ -274,7 +280,7 @@ Pane::reallocate (Gtk::Allocation const & alloc) /* Move on to next *visible* child */ while (++next != children.end()) { - if (next->w->is_visible()) { + if ((*next)->w->is_visible()) { break; } } @@ -291,7 +297,7 @@ Pane::reallocate (Gtk::Allocation const & alloc) } Gtk::Requisition cr; - child->w->size_request (cr); + (*child)->w->size_request (cr); if (horizontal) { child_alloc.set_width ((gint) floor (remaining * fract)); @@ -305,15 +311,15 @@ Pane::reallocate (Gtk::Allocation const & alloc) ypos += child_alloc.get_height (); } - if (child->minsize) { + if ((*child)->minsize) { if (horizontal) { - child_alloc.set_width (max (child_alloc.get_width(), child->minsize)); + child_alloc.set_width (max (child_alloc.get_width(), (*child)->minsize)); } else { - child_alloc.set_height (max (child_alloc.get_height(), child->minsize)); + child_alloc.set_height (max (child_alloc.get_height(), (*child)->minsize)); } } - child->w->size_allocate (child_alloc); + (*child)->w->size_allocate (child_alloc); if (next == children.end()) { /* done, no more children, no need for a divider */ @@ -362,8 +368,8 @@ Pane::on_expose_event (GdkEventExpose* ev) for (child = children.begin(), div = dividers.begin(); child != children.end(); ++child) { - if (child->w->is_visible()) { - propagate_expose (*(child->w), ev); + if ((*child)->w->is_visible()) { + propagate_expose (*((*child)->w), ev); } if (div != dividers.end()) { @@ -392,7 +398,7 @@ Pane::handle_release_event (GdkEventButton* ev, Divider* d) d->dragging = false; if (did_move && !children.empty()) { - children.front().w->queue_resize (); + children.front()->w->queue_resize (); did_move = false; } @@ -418,16 +424,16 @@ Pane::constrain_fract (Dividers::size_type div, float fract) const float size = horizontal ? get_allocation().get_width() : get_allocation().get_height(); // TODO: optimize: cache in Pane::on_size_request - Gtk::Requisition prev_req(children.at (div).w->size_request ()); - Gtk::Requisition next_req(children.at (div + 1).w->size_request ()); + Gtk::Requisition prev_req(children.at (div)->w->size_request ()); + Gtk::Requisition next_req(children.at (div + 1)->w->size_request ()); float prev = divider_width + (horizontal ? prev_req.width : prev_req.height); float next = divider_width + (horizontal ? next_req.width : next_req.height); - if (children.at (div).minsize) { - prev = children.at (div).minsize; + if (children.at (div)->minsize) { + prev = children.at (div)->minsize; } - if (children.at (div + 1).minsize) { - next = children.at (div + 1).minsize; + if (children.at (div + 1)->minsize) { + next = children.at (div + 1)->minsize; } if (size * fract < prev) { @@ -598,7 +604,7 @@ Pane::forall_vfunc (gboolean include_internals, GtkCallback callback, gpointer c for (Children::iterator c = children.begin(); c != children.end(); ) { Children::iterator next = c; ++next; - callback (c->w->gobj(), callback_data); + callback ((*c)->w->gobj(), callback_data); c = next; } -- cgit v1.2.3