Branch: refs/heads/master Author: Colomban Wendling ban@herbesfolles.org Committer: Colomban Wendling ban@herbesfolles.org Date: Mon, 31 Jul 2023 11:40:20 UTC Commit: 35d556ede85efb358d1cac156dd0fc8d44b20201 https://github.com/geany/geany/commit/35d556ede85efb358d1cac156dd0fc8d44b202...
Log Message: ----------- 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.
Modified Paths: -------------- src/sidebar.c
Modified: src/sidebar.c 27 lines changed, 19 insertions(+), 8 deletions(-) =================================================================== @@ -1313,24 +1313,28 @@ static void document_action(GeanyDocument *doc, gint action) }
-static void on_openfiles_document_action_apply(GtkTreeModel *model, GtkTreeIter iter, gint action) +/* Collects all documents under a given iter, filling @doc_array */ +static void on_openfiles_document_action_collect(GtkTreeModel *model, GtkTreeIter *iter, + GPtrArray *doc_array) { GeanyDocument *doc; - gtk_tree_model_get(model, &iter, DOCUMENTS_DOCUMENT, &doc, -1); + gtk_tree_model_get(model, iter, DOCUMENTS_DOCUMENT, &doc, -1); if (doc) { - document_action(doc, action); + g_ptr_array_add(doc_array, doc); } else { /* parent item selected */ GtkTreeIter child; - gint i = gtk_tree_model_iter_n_children(model, &iter) - 1;
- while (i >= 0 && gtk_tree_model_iter_nth_child(model, &child, &iter, i)) + if (gtk_tree_model_iter_children(model, &child, iter)) { - on_openfiles_document_action_apply(model, child, action); - i--; + do + { + on_openfiles_document_action_collect(model, &child, doc_array); + } + while (gtk_tree_model_iter_next(model, &child)); } } } @@ -1344,7 +1348,14 @@ static void on_openfiles_document_action(GtkMenuItem *menuitem, gpointer user_da gint action = GPOINTER_TO_INT(user_data);
if (gtk_tree_selection_get_selected(selection, &model, &iter)) - on_openfiles_document_action_apply(model, iter, action); + { + GPtrArray *doc_array = g_ptr_array_new(); + + on_openfiles_document_action_collect(model, &iter, doc_array); + for (guint i = 0; i < doc_array->len; i++) + document_action(g_ptr_array_index(doc_array, i), action); + g_ptr_array_free(doc_array, TRUE); + } }
-------------- This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).