Yet another pane pain: allow deleting children using forall_vfunc

We not only need to make sure the iterator remains valid, but also
the object pointed to.

Valgrind trace:
Invalid read of size 8
 Gtkmm2ext::Pane::forall_vfunc(int, void (*)(_GtkWidget*, void*), void*) (pane.cc:617)
 Gtk::Container_Class::forall_vfunc_callback(_GtkContainer*, int, void (*)(_GtkWidget*, void*), void*)
 gtk_container_destroy (gtkcontainer.c:1073)
 g_closure_invoke (gclosure.c:804)
...
 g_object_run_dispose (gobject.c:1084)

diff --git a/libs/gtkmm2ext/gtkmm2ext/pane.h b/libs/gtkmm2ext/gtkmm2ext/pane.h
index d8367b36f6ef7dd5b8999b1c580dc4d166d98df2..88eb82041bcc748d5d452fde42d82b6129a519f8 100644 (file)
--- a/libs/gtkmm2ext/gtkmm2ext/pane.h
+++ b/libs/gtkmm2ext/gtkmm2ext/pane.h
@@ -22,6 +22,7 @@
 
 #include <vector>
 #include <algorithm>
+#include <boost/shared_ptr.hpp>
 
 #include <stdint.h>
 
@@ -55,7 +56,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<Child*> Children;
+       typedef std::vector<boost::shared_ptr<Child> > Children;
 
        Pane (bool horizontal);
        ~Pane();

diff --git a/libs/gtkmm2ext/pane.cc b/libs/gtkmm2ext/pane.cc
index 19e80aebb9ba6289937090f4b65e8ffb5bdcd894..76e68de5a29954f9ecfa40ff6bbc535f003f4533 100644 (file)
--- a/libs/gtkmm2ext/pane.cc
+++ b/libs/gtkmm2ext/pane.cc
@@ -51,9 +51,10 @@ 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 ();
-               delete (*c);
+               if ((*c)->w) {
+                       (*c)->w->remove_destroy_notify_callback ((*c).get());
+                       (*c)->w->unparent ();
+               }
        }
        children.clear ();
 }
@@ -158,8 +159,8 @@ Pane::handle_child_visibility ()
 void
 Pane::on_add (Widget* w)
 {
-       children.push_back (new Child (this, w, 0));
-       Child* kid = children.back ();
+       children.push_back (boost::shared_ptr<Child> (new Child (this, w, 0)));
+       Child* kid = children.back ().get();
 
        w->set_parent (*this);
        /* Gtkmm 2.4 does not correctly arrange for ::on_remove() to be called
@@ -190,9 +191,8 @@ Pane::child_destroyed (Gtk::Widget* w)
                if ((*c)->w == w) {
                        (*c)->show_con.disconnect ();
                        (*c)->hide_con.disconnect ();
-                       Child* kid = *c;
+                       (*c)->w = NULL; // mark invalid
                        children.erase (c);
-                       delete kid;
                        break;
                }
        }
@@ -206,11 +206,10 @@ Pane::on_remove (Widget* w)
                if ((*c)->w == w) {
                        (*c)->show_con.disconnect ();
                        (*c)->hide_con.disconnect ();
-                       w->remove_destroy_notify_callback (*c);
+                       w->remove_destroy_notify_callback ((*c).get());
                        w->unparent ();
-                       Child* kid = *c;
+                       (*c)->w = NULL; // mark invalid
                        children.erase (c);
-                       delete kid;
                        break;
                }
        }
@@ -610,12 +609,11 @@ Pane::forall_vfunc (gboolean include_internals, GtkCallback callback, gpointer c
        /* since the callback could modify the child list(s), make sure we keep
         * the iterators safe;
         */
-
-       for (Children::iterator c = children.begin(); c != children.end(); ) {
-               Children::iterator next = c;
-               ++next;
-               callback ((*c)->w->gobj(), callback_data);
-               c = next;
+       Children kids (children);
+       for (Children::const_iterator c = kids.begin(); c != kids.end(); ++c) {
+               if ((*c)->w) {
+                       callback ((*c)->w->gobj(), callback_data);
+               }
        }
 
        if (include_internals) {