The problem appears to be that foreach does not stop when you delete the last row in the last iteration; instead it tries to keep iterating over the end of the list.  Doesn't look like something glade related.  Over to gtk+.

Could you also attach "bug.glade", so I can get the test case running?

Created attachment 68949 [details]
The glade file of the bug

Here is it.

Thanks, that helped ;)


The problem is that gtk_list_store_remove() will change the iter (it sets iter->stamp to zero, so that the iterator is marked as invalid).  But the python marshal function for GtkTreeModelForeachFuncs copies the iter, and the iter is not copied back after running the user defined foreach func.  So gtk_tree_model_foreach() does not get the updated, now invalid, iterator and gtk_tree_model_iter_next() still succeeds.

According to Johan it's probably better to not copy the iter at all.  Anyway, reassinging back to PyGtk.

Ok, I know that that wasn't the right way to remove some items (I switched to the right way right after), but it shoudn't segfault, as far as I know :)

After a quick look it appears that there are several places where the TreeIter should not be copied:

pygtk_cell_data_func_marshal, 
pygtk_tree_selection_foreach_marshal, 
pygtk_tree_sortable_sort_cb, pygtk_tree_foreach_marshal, pygtk_tree_model_filter_visible_cb, 
pygtk_filter_modify_func_marshal, 
pygtk_set_search_equal_func_marshal, 
pygtk_set_row_separator_func_marshal

All these should probably wrap the incoming TreeIter as follows:

pyg_boxed_new(GTK_TYPE_TREE_ITER, iter, FALSE, FALSE)

It would be good to have a test for this, so we can verify that this continues to work in the future. Since apparently nobody tried to do something that changes the iter in any of these callbacks.

* gtk/gtktreeview.override (pygtk_cell_data_func_marshal)
	(pygtk_tree_selection_foreach_marshal, pygtk_tree_sortable_sort_cb)
	(pygtk_tree_foreach_marshal, pygtk_tree_model_filter_visible_cb)
	(pygtk_filter_modify_func_marshal)
	(pygtk_set_search_equal_func_marshal): Don't copy TreeIter when passing
	to callback. #347273 (Stefano Maggiolo)

Checking in gtk/gtktreeview.override;
/cvs/gnome/pygtk/gtk/gtktreeview.override,v  <--  gtktreeview.override
new revision: 1.70; previous revision: 1.69
done


Now back to GTK to fix the bug that the foreach function doesn't properly handle the side effect of gtk_list_store_remove on the TreeIter.

e.g.

model = gtk.ListStore(str)
model.append(['a'])
model.append(['b'])
model.append(['c'])
model.append(['d'])
model.append(['e'])
def f(m,p,i):
  print m[i][0], p
  m.remove[i]
  return False
model.foreach(f)

produces:

a (0,)
c (1,)
e (2,)

For the record, pyg_boxed_new(GTK_TYPE_TREE_ITER, iter, FALSE, FALSE) is slightly dangerous and I wish we didn't need to use it; it creates a PyGBoxed wrapper for a boxed type it doesn't own.  It is possible for the callback to keep the boxed wrapper around, even after the original boxed object itself has been destroyed, with potentially dangerous effects if the boxed wrapper is used later.

This problem already appeared in signal handlers, and jpe solved it by copying all boxed types after the signal handler in case their refcount > 1, in pygtype.c pyg_signal_class_closure_marshal():

    /* Copy boxed values if others ref them, this needs to be done regardless of
       exception status. */
    len = PyTuple_Size(params);    
    for (i = 0; i < len; i++) {
	PyObject *item = PyTuple_GetItem(params, i);
	if (item != NULL && PyObject_TypeCheck(item, &PyGBoxed_Type)
	    && item->ob_refcnt != 1) {
	    PyGBoxed* boxed_item = (PyGBoxed*)item;
	    if (!boxed_item->free_on_dealloc) {
		boxed_item->boxed = g_boxed_copy(boxed_item->gtype, boxed_item->boxed);
		boxed_item->free_on_dealloc = TRUE;
	    }
	}
    }

It might be worth to export a pygobject API, like pyg_boxed_copy_if_alive(), which would copy the boxed if refcnt > 1; then we'd need to call this function for all boxed types created with pyg_boxed_new(TYPE, boxed, FALSE, FALSE) and passed into user code that might keep them alive.

Grr! I knew this was a bad idea :|

I am almost sure this change is causing this bug: https://launchpad.net/distros/ubuntu/+source/revelation/+bug/56744

The affected code is:

	def get_selected(self):
		"Get a list of currently selected rows"

		list = []
		self.selection.selected_foreach(lambda model, path, iter: list.append(iter))

		return list

(where self.selection is a gtk.TreeSelection object)

The PyGTk wrapper was modified in the course of fixing this bug:

1.70         (finlay   18-Jul-06):     py_iter = pyg_boxed_new(GTK_TYPE_TREE_ITER, iter, FALSE, FALSE);

So what I predicted a few months ago is actually happening: python code is keeping an iterator around, but this is an PyGtkTreeIter wrapper that contains shared pointer to a GtkTreeIter that is destroyed when it goes out of scope.

I have confirmed then fixed the bug.

Checking in ChangeLog;
/cvs/gnome/pygtk/ChangeLog,v  <--  ChangeLog
new revision: 1.1615; previous revision: 1.1614
done
Checking in gtk/gtkobject-support.c;
/cvs/gnome/pygtk/gtk/gtkobject-support.c,v  <--  gtkobject-support.c
new revision: 1.23; previous revision: 1.22
done
Checking in gtk/gtktreeview.override;
/cvs/gnome/pygtk/gtk/gtktreeview.override,v  <--  gtktreeview.override
new revision: 1.74; previous revision: 1.73
done
Checking in gtk/pygtk-private.h;
/cvs/gnome/pygtk/gtk/pygtk-private.h,v  <--  pygtk-private.h
new revision: 1.34; previous revision: 1.33
done
Checking in gtk/pygtkcellrenderer.c;
/cvs/gnome/pygtk/gtk/pygtkcellrenderer.c,v  <--  pygtkcellrenderer.c
new revision: 1.13; previous revision: 1.12
done