Fix crash closing directory from the openfiles sidebar
commit35d556ede85efb358d1cac156dd0fc8d44b20201
authorColomban Wendling <ban@herbesfolles.org>
Sun, 30 Jul 2023 21:41:02 +0000 (30 23:41 +0200)
committerColomban Wendling <ban@herbesfolles.org>
Mon, 31 Jul 2023 11:40:20 +0000 (31 13:40 +0200)
tree84b25c1856dcc839456d0505a97d0b8d5ee8406e
parent8bf521cd5d99ebb1f1e6942ca5b35e42b4d11dfb
Fix crash closing directory from the openfiles sidebar

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.
src/sidebar.c