When the openfiles sidebar shows documents as a tree, closing a document can
lead to sever re-layout of the view (e.g. collapsing directory nodes together).
This makes walking the tree and closing documents at the same time highly
tricky, as nodes might be shifting as we go.
This lead to invalid memory access, and unexpected results, when closing some
tree structures.
One example of Valgrind showing how bad things are:
==917061== Invalid read of size 8
==917061== at 0x5B31345: g_node_nth_child (gnode.c:1052)
==917061== by 0x50A7361: gtk_tree_store_iter_nth_child
(gtktreestore.c:793)
==917061== by 0x491DB76: on_openfiles_document_action_apply
(sidebar.c:1330)
==917061== by 0x491DC0C: on_openfiles_document_action (sidebar.c:1347)
==917061== by 0x5A833AF: g_closure_invoke (gclosure.c:832)
==917061== by 0x5A96075: signal_emit_unlocked_R.isra.0 (gsignal.c:3796)
==917061== by 0x5A9CBF4: g_signal_emit_valist (gsignal.c:3549)
==917061== by 0x5A9CDBE: g_signal_emit (gsignal.c:3606)
==917061== by 0x50D7833: gtk_widget_activate (gtkwidget.c:7845)
==917061== by 0x4F8A935: gtk_menu_shell_activate_item
(gtkmenushell.c:1375)
==917061== by 0x4F8AC70: gtk_menu_shell_button_release
(gtkmenushell.c:791)
==917061== by 0x4DFBCB3: _gtk_marshal_BOOLEAN__BOXEDv
(gtkmarshalers.c:130)
==917061== Address 0x9bcdc20 is 32 bytes inside a block of size 40
free'd
==917061== at 0x484317B: free (vg_replace_malloc.c:872)
==917061== by 0x5B3068B: g_nodes_free (gnode.c:123)
==917061== by 0x5B3068B: g_node_destroy (gnode.c:143)
==917061== by 0x50AA8F2: gtk_tree_store_remove (gtktreestore.c:1229)
==917061== by 0x491F851: sidebar_openfiles_remove_iter (sidebar.c:959)
==917061== by 0x491F8AE: openfiles_remove (sidebar.c:972)
==917061== by 0x491FA6C: sidebar_remove_document (sidebar.c:1027)
==917061== by 0x48D0B5B: remove_page (document.c:733)
==917061== by 0x48D29C0: document_remove_page (document.c:787)
==917061== by 0x48D29FC: document_close (document.c:695)
==917061== by 0x491DAED: document_action (sidebar.c:1299)
==917061== by 0x491DB47: on_openfiles_document_action_apply
(sidebar.c:1322)
==917061== by 0x491DB9E: on_openfiles_document_action_apply
(sidebar.c:1332)
==917061== Block was alloc'd at
==917061== at 0x48407B4: malloc (vg_replace_malloc.c:381)
==917061== by 0x5B2C678: g_malloc (gmem.c:130)
==917061== by 0x5B45011: g_slice_alloc (gslice.c:1074)
==917061== by 0x5B305BD: g_node_new (gnode.c:110)
==917061== by 0x50AAF0B: gtk_tree_store_insert_before
(gtktreestore.c:1375)
==917061== by 0x491E725: tree_add_new_dir (sidebar.c:654)
==917061== by 0x491E89A: get_parent_for_file (sidebar.c:840)
==917061== by 0x491E995: sidebar_openfiles_add_iter (sidebar.c:869)
==917061== by 0x491F5C7: sidebar_openfiles_add (sidebar.c:901)
==917061== by 0x48D0CE4: document_create (document.c:662)
==917061== by 0x48D3840: document_open_file_full (document.c:1349)
==917061== by 0x48D3C81: document_open_file (document.c:914)
Fix this by decoupling tree walking from closing documents. We now do two
passes: first we collect documents to work on walking the tree as before, and
only then we perform the action on each node of the list.
Fixes #3527.
You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3535
-- Commit Summary --
* Fix crash closing directory from the openfiles sidebar
-- File Changes --
M src/sidebar.c (29)
-- Patch Links --
https://github.com/geany/geany/pull/3535.patch
https://github.com/geany/geany/pull/3535.diff
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3535
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3...@github.com>
