GNOME Bugzilla – Bug 346800
Rework sort/filter models to use indices to parents instead of pointers
Last modified: 2011-02-04 16:11:11 UTC
HEAD from today. I feel like I've reported this before but I can't find the bug. Attempt to remove filter function 0xe15dc0 user data 0x8e898f8, but no such filter has been added NameOwnerChanged org.gnome.Rhythmbox ->:1.7 Gtk-ERROR **: file gtktreemodelfilter.c: line 2237 (gtk_tree_model_filter_get_path): assertion failed: (i < level->array->len) aborting... Using host libthread_db library "/lib/libthread_db.so.1". `shared object read from target memory' has disappeared; keeping its symbols. [Thread debugging using libthread_db enabled] [New Thread -1208838448 (LWP 12410)] 0x00abe402 in __kernel_vsyscall ()
+ Trace 69221
Thread 1 (Thread -1208838448 (LWP 12410))
*** Bug 346885 has been marked as a duplicate of this bug. ***
I've seen this on 0.9.5 as well, but it happens only occasionally (it will fail 10 times in a row, but when I try hours later, it works.) Only thing I can think that would be different between runs is scanned local shares that it's found... gtk2-2.10.0, if that makes a difference.
I'm not sure if it's what caused the crash, but we're not holding the GDK lock in the code in the backtrace. It's a bit tricky because the entry-added signal may be emitted with or without the lock being held, depending on what made it happen. And we can't just grab it, because it's a GMuted which may not be recursive. Urk.
Another datapoint: doesn't crash for me when not connected to the network and no audio CD is in the drive.
From watching the display update when it crashes and when it doesn't, it may be related to updates to the sources when scanning the library; it crashes for me about 2/3 of the way through scanning the library of local files. (And yet, other times it scans them all fine.)
It seems that there is an interaction between daap sources and setting the visibility of the missing files source. Tracking this down is complicated a bit by the way that visibility is handed for the missing files and import errors sources.
Created attachment 68827 [details] [review] patch to clean up hide when empty behavior This makes hidden-when-empty a property on the source. This cleans up the API a bit and saves a bit of memory. Look OK to commit?
Created attachment 68831 [details] [review] patch Looks like a reentrancy issue to me. We can avoid this by deferring the visibility notification into an idle. So far this patch seems to fix it for me. Bill can you test it?
Sorry, no that doesn't fix it. Might still be a good thing to do though.
*** Bug 339631 has been marked as a duplicate of this bug. ***
Created attachment 68832 [details] [review] patch Take the gdk lock when doing the refilter iteration. Seems to work for me so far (after 30 restarts or so).
Hm, doesn't apply striaght to 0.9.5. Will prod it and try.
The most recent patch crashes in a slightly different way after many successful tries. Using host libthread_db library "/lib/libthread_db.so.1". `shared object read from target memory' has disappeared; keeping its symbols. [Thread debugging using libthread_db enabled] [New Thread -1208944096 (LWP 1546)] [New Thread 52984736 (LWP 1554)] 0x00abe402 in __kernel_vsyscall ()
+ Trace 69291
Thread 1 (Thread -1208944096 (LWP 1546))
Still crashes with the patch for me as well.
Regardless of whether it actually fixes anything, the patch looks OK to me. Seems I need to do something about my editor leaving trailing whitespace everywhere, too.
Committed the patch without the gdk lock.
*** Bug 347542 has been marked as a duplicate of this bug. ***
*** Bug 347757 has been marked as a duplicate of this bug. ***
*** Bug 347875 has been marked as a duplicate of this bug. ***
I'm pretty sure this only happens with gtk+ 2.10.0. Testing with 2.8.20 is ok.
I want to add, that while I was on dapper with gtk+ 2.8, it was fine. After upgrading to edgy I'm not able to start it without crash. Gtk+ 2.10.1 didn't solved it (if it's Gtk+'s fault).
*** Bug 348991 has been marked as a duplicate of this bug. ***
*** Bug 349058 has been marked as a duplicate of this bug. ***
*** Bug 349101 has been marked as a duplicate of this bug. ***
*** Bug 349093 has been marked as a duplicate of this bug. ***
*** Bug 349317 has been marked as a duplicate of this bug. ***
*** Bug 349376 has been marked as a duplicate of this bug. ***
*** Bug 349647 has been marked as a duplicate of this bug. ***
*** Bug 342938 has been marked as a duplicate of this bug. ***
Looks like a reference counting problem (or perhaps some bizarre memory corruption). ==9426== ==9426== Invalid read of size 4 ==9426== at 0x42E4997: gtk_tree_model_filter_row_changed (gtktreemodelfilter.c:1296) ==9426== by 0x42E4D75: gtk_tree_model_filter_refilter_helper (gtktreemodelfilter.c:3283) ==9426== by 0x42DFDA7: gtk_tree_model_foreach_helper (gtktreemodel.c:1569) ==9426== by 0x42DFDE6: gtk_tree_model_foreach_helper (gtktreemodel.c:1575) ==9426== by 0x42E059C: gtk_tree_model_foreach (gtktreemodel.c:1615) ==9426== by 0x42E16F4: gtk_tree_model_filter_refilter (gtktreemodelfilter.c:3303) ==9426== by 0x8097403: visibility_notify_cb (rb-sourcelist.c:830) ==9426== by 0x4E294CA: g_cclosure_marshal_VOID__PARAM (gmarshal.c:531) ==9426== by 0x4E1CA2A: g_closure_invoke (gclosure.c:490) ==9426== by 0x4E2CEBC: signal_emit_unlocked_R (gsignal.c:2438) ==9426== by 0x4E2E3B8: g_signal_emit_valist (gsignal.c:2197) ==9426== by 0x4E2E568: g_signal_emit (gsignal.c:2241) ==9426== Address 0x5894298 is 32 bytes inside a block of size 252 free'd ==9426== at 0x401C139: free (vg_replace_malloc.c:233) ==9426== by 0x4BA589A: cairo_surface_destroy (in /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4BA8FF9: (within /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4BBEA98: (within /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4BA5184: (within /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4B9A088: (within /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4B94763: cairo_show_glyphs (in /usr/lib/libcairo.so.2.9.0) ==9426== by 0x4B500E3: (within /usr/lib/libpangocairo-1.0.so.0.1201.2) ==9426== by 0x4B724C4: pango_renderer_draw_glyphs (in /usr/lib/libpango-1.0.so.0.1201.2) ==9426== by 0x4B4F61E: pango_cairo_show_glyph_string (in /usr/lib/libpangocairo-1.0.so.0.1201.2) ==9426== by 0x4441FD8: gdk_pango_renderer_draw_glyphs (gdkpango.c:244) ==9426== by 0x4B724C4: pango_renderer_draw_glyphs (in /usr/lib/libpango-1.0.so.0.1201.2) Gtk-ERROR **: file gtktreemodelfilter.c: line 2237 (gtk_tree_model_filter_get_path): assertion failed: (i < level->array->len) aborting...
Revision 1.34 of gtktreemodelfilter.c doesn't crash, revision 1.35 does. The diff between the two is huge, and I don't understand any of the code all that well. Fun.
*** Bug 350078 has been marked as a duplicate of this bug. ***
Created attachment 70290 [details] test case This program reproduces the problem at around iteration 17 (for me - it depends on memory allocation behaviour). The exact tree store manipulations probably don't matter; I was just trying to do something that vaguely resembled what the RB source list does.
Created attachment 70291 [details] [review] gtk+ patch This patch to gtk+ fixes the problem for me, both in the above test program and in rhythmbox. GtkTreeModelFilter needs to update all of a FilterLevel's children when it inserts a new element into level->array, as doing so can result in the whole array moving. In gtk_tree_model_filter_fetch_child, it was trying to only update the children after the point where a new FilterElt was inserted.
Reassigning to gtk+, obsoleting patches. It looks like the bug is present on the gtk-2-8 branch, but it doesn't get triggered in this case for some reason.
Confirming patch works here.
*** Bug 350184 has been marked as a duplicate of this bug. ***
*** Bug 350279 has been marked as a duplicate of this bug. ***
*** Bug 350361 has been marked as a duplicate of this bug. ***
*** Bug 350478 has been marked as a duplicate of this bug. ***
*** Bug 350604 has been marked as a duplicate of this bug. ***
Ubuntu bug about that: https://launchpad.net/distros/ubuntu/+source/rhythmbox/+bug/55866 That bug has quite some duplicates and a patch, is somebody looking to it?
that patch fixes the issue with rhythmbox for the submitter of the ubuntu bug too
*** Bug 350871 has been marked as a duplicate of this bug. ***
*** Bug 350874 has been marked as a duplicate of this bug. ***
*** Bug 351019 has been marked as a duplicate of this bug. ***
*** Bug 351017 has been marked as a duplicate of this bug. ***
*** Bug 351189 has been marked as a duplicate of this bug. ***
*** Bug 351221 has been marked as a duplicate of this bug. ***
Kris, can you comment on the patch in #34, please ?
*** Bug 351382 has been marked as a duplicate of this bug. ***
(In reply to comment #34) > This patch to gtk+ fixes the problem for me, both in the above test program and > in rhythmbox. GtkTreeModelFilter needs to update all of a FilterLevel's > children when it inserts a new element into level->array, as doing so can > result in the whole array moving. In gtk_tree_model_filter_fetch_child, it was > trying to only update the children after the point where a new FilterElt was > inserted. Oh, really good catch. (In reply to comment #35) > Reassigning to gtk+, obsoleting patches. It looks like the bug is present on > the gtk-2-8 branch, but it doesn't get triggered in this case for some reason. I think there's a good chance that is triggered now that GArray has been moved over to GArray. Anyway, I will commit this on 2-8 as well.
(Oh, and sorry that this took so long, but I was busy with other things ...).
looking at the patch/code, the problem here really is that the array elements store pointers to other array elements. those pointers can become invalid with *any* kind of array modification, so the only proper fix here (at least long term) is to convert FilterElt.children to an index, rather than a pointer.
Needs to happen for the sort model too then. Putting on the 2.12 list.
*** Bug 351676 has been marked as a duplicate of this bug. ***
*** Bug 351722 has been marked as a duplicate of this bug. ***
*** Bug 351807 has been marked as a duplicate of this bug. ***
*** Bug 351816 has been marked as a duplicate of this bug. ***
*** Bug 351840 has been marked as a duplicate of this bug. ***
*** Bug 351841 has been marked as a duplicate of this bug. ***
*** Bug 351878 has been marked as a duplicate of this bug. ***
*** Bug 351871 has been marked as a duplicate of this bug. ***
*** Bug 351988 has been marked as a duplicate of this bug. ***
*** Bug 351950 has been marked as a duplicate of this bug. ***
Created attachment 75681 [details] [review] rework sort model This patch reworks the sort model to use an index to the parent element instead of a pointer. I've shortly tested it with testtreesort and it seems to work; however I want to do some extensive testing before I commit. The fix for the filter model will look likewise, but still have to cook that one up.
*** Bug 378223 has been marked as a duplicate of this bug. ***
Kristian, did you manage to test your patch more extensively?
This patch is not going in until I have written an extensive test suite for tree models and I also reworked the filter model.
Am I right in thinking the following stack trace is similar / the same as this bug? (GTK 2.11.6) Strangely, I can only ever reporoduce this when running under electric-fence, although electric-fence doesn't etect any badness. The app is gschem, part of the GPL EDA suite (gEDA). Are any app-level workarounds / coding practices which would prevent it? Gtk-ERROR **: file /build/buildd/gtk+2.0-2.11.6/gtk/gtktreemodelfilter.c: line 2236 (gtk_tree_model_filter_get_path): assertion failed: (i < level->array->len) aborting... Program received signal SIGTRAP, Trace/breakpoint trap.
+ Trace 154530
Thread NaN (LWP 16121)
Punting until I finished the test suite.
Ping! Is this bug still present?
Yes, I really want to get a test suite in place for the sort and filter models first before I continue committing changes there ...
Marking filter model bugs I want to look into shortly as ASSIGNED. (I am working into getting the test suite as mentioned in comment 73 into place.).
The test suite is now there. Both the sort and filter models have now been converted. Fixed on master.