From 6b5dce2c66554df2243d62ca06c93b9c9eefc8fa Mon Sep 17 00:00:00 2001
From: Robin Gareus <robin@gareus.org>
Date: Fri, 20 Jan 2017 03:13:41 +0100
Subject: [PATCH] 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)
---
 libs/gtkmm2ext/gtkmm2ext/pane.h |  3 ++-
 libs/gtkmm2ext/pane.cc          | 30 ++++++++++++++----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/libs/gtkmm2ext/gtkmm2ext/pane.h b/libs/gtkmm2ext/gtkmm2ext/pane.h
index d8367b36f6..88eb82041b 100644
--- 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 19e80aebb9..76e68de5a2 100644
--- 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) {