Hi, I'm investigating an issue from a Debian user, where its thunar process constantly access files in /proc. It seems that the file monitoring tracks /proc folder, and, as it constantly changes (which is normal for files in /proc), it warns Thunar, which do some activity. On its machine, it seems quite unusable (lots of files, which are not "normal" files etc., and it's not a powerful machine). First solution is not to go to /proc with a file manager, which is indeed a solution but not really acceptable. We shouldn't have to force user to not do something (or at least in that case we should prevent him open /proc). But the real problem is that the monitoring doesn't stop when leaving /proc. This is only noticeable using Tree View side pane. If you go to /proc using tree view, then leave it, thunar/gamin (or fam) will still track the content. You can watch this by running thunar through strace | grep proc, or using the debug stuff in gamin (kill -USR2, then watching /prob/gamin_debug_xxxx). This is not reproducible using the shortcut Side Pane, so I guess when you leave a folder in the main view area, it's really closed, while if you close it on the tree view (clicking the arrow) it's not really closed. I'm currently investigating thunar sources to see how this could be changed, and may come with a patch, but I'm not that familiar with thunar sources, so if you have time to look at it, it'd be really great. Oh and btw this is on 0.9.0. Cheers, and thanks for your work!
And this one looks like bug #2502
Created attachment 1991 Patch v1 Ok, here's a first attempt to fix this. The patch does a few things: * Implement a filter in the model. This is needed to make node_ref and node_unref work properly (GtkTreeModelFilter keeps references). * It removes nodes from the tree when a node is collapsed (unreffed). * Because the filter was reffing a lot, if also needed quite a bit of code changes to work properly: - Loading a node happens in 2 stages: when it's added as a subfolder in the tree (collapsed) we only check if it needs a dummy node. When it expands the subfolders are loaded. - Hidden files are added in a separate list in the parent node. I did this because the (for gtk) invisible files in the tree added a lot of complexity in the gtktreemodel iface functions, and also because we only 'care' about the file, so no need to keep other slices around. Please try it and report problems. It also adds some lines to thunar-vfs to dump the number of file handles in the file monitor, so you can watch if folders are properly unmonitored when you close a tree.
Created attachment 1994 Patch v2 Updated version of the pervious patch with the following changes: * Fix lock in fam when removing a lot a quests (collapse a large tree). * Add an unload idle for the entire tree, not just the tree item. This is a lot more efficient if multiple sub folders are collapsed. * Minor fixes.
Last patch has a bug when folder items are collapsed/expanded. Will fix that later.
Created attachment 1998 Patch v3 Final version of the patch. Changes compared to the previous patch: * Drop the item state, was buggy. Folder are now loaded like they were. * The global unload idle is changed to a timeout of 500ms. This gives the treeview the needed time to unref all the nodes, so the unload tree traverse (wich is pre order), works for efficiently. * Add shared function descriptions. * Various cleanups to make the patch as small as possible. * Some extra debug check to make sure everything works as expected. Benny, can you take a look at this patch?
Created attachment 1999 Patch v4 Fixed some typos and restored some origional code. No functional changes.
Created attachment 2000 Patch v5 Ok a bit more changes: * Added a hack in thunar-folder to skip ThunarFile reloads when the file has a ref count of 1. I know this is ugly, but it greatly reduces the cpu usage of thunar when (for example) the /proc folder is expanded. Tested it a bit, seems to work fine. * Add a check in thunar_tree_model_file_changed() to only traverse the tree if the changed file is a folder (also reduced the cpu usage). * Improve locking of the ThunarVfsMonitor, so it's better resistant when removing a lot of files from the monitor. I know the hack is ugly, but since it makes thunar response again in folder like /proc (in combination with the treeview) and i can't think of another way to check if the file is actually used (visible) in thunar, i think we should (atleast) try it.
(In reply to comment #7) > I know the hack is ugly, but since it makes thunar response again in folder > like /proc (in combination with the treeview) and i can't think of another way > to check if the file is actually used (visible) in thunar, i think we should > (atleast) try it. BTW, I don't know about fam, but gamin ignores /proc and mounts like that. It's safer to still prevent thunar crashes though.
(In reply to comment #7) > Created an attachment (id=2000) [details] > Patch v5 > > Ok a bit more changes: > > * Added a hack in thunar-folder to skip ThunarFile reloads when the file has a > ref count of 1. I know this is ugly, but it greatly reduces the cpu usage of > thunar when (for example) the /proc folder is expanded. Tested it a bit, seems > to work fine. Are there cases where files have a refcount of 1 and reloads *need* to be taken into account? What happens if you don't use the tree view pane?
(In reply to comment #8) > BTW, I don't know about fam, but gamin ignores /proc and mounts like that. It's > safer to still prevent thunar crashes though. That's not true. Gamin is just another backend for the fam api. Using gamin here and it goes crazy when i expand the /proc folder in thunar.
(In reply to comment #9) > Are there cases where files have a refcount of 1 and reloads *need* to be taken > into account? What happens if you don't use the tree view pane? It seems to work fine here, but we should just give it some wider testing. I've tried it with the bookmark- and no side pane and that seems to work, will give it some more testing later today.
(In reply to comment #10) > (In reply to comment #8) > > BTW, I don't know about fam, but gamin ignores /proc and mounts like that. It's > > safer to still prevent thunar crashes though. > > That's not true. Gamin is just another backend for the fam api. Using gamin > here and it goes crazy when i expand the /proc folder in thunar. That's a bug in gamin (which I could reproduce at one time but cant now). See: ,----[ gamin-0.1.9/server/gam_excludes.c ] | static char *static_excludes[] = { | #ifdef HAVE_LINUX | "/media/*", | "/mnt/*", | "/dev/*", | "/proc/*", | #endif | NULL | }; `---- I've reported a bug against gamin package in debian (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=478742 which originates from the debian thunar bug) but it seems now solved. Anyway, the intended behavior is to ignore those paths. Cheers,
Well if gamin _should_ actually do that, we can drop the ref count hack. Jannis can you still reproduce the lockup when collapsing nodes in the treeview?
(In reply to comment #13) > Well if gamin _should_ actually do that, we can drop the ref count hack. I'll ask the original reporter if he can still reproduce the bug with thunar 0.9.0 and gamin.
As an alternative we can also add a check in the Thunar monitor to skip /proc and /dev emits on file-change (but keep emitting file-added and file-removed). Anyway, when Jannis tried the patch and can't reproduce the crash when collapsing nodes, I suggest we commit the patch without the ref_count hack and continue the heavy gam/fam usage in a separate bug.
(In reply to comment #15) > As an alternative we can also add a check in the Thunar monitor to skip /proc > and /dev emits on file-change (but keep emitting file-added and file-removed). > > Anyway, when Jannis tried the patch and can't reproduce the crash when > collapsing nodes, I suggest we commit the patch without the ref_count hack and > continue the heavy gam/fam usage in a separate bug. It doesn't crash here with your patch but it polls /proc/* like crazy. I'd say we apply your patch and I'll add temporary code to ignore the excludes gamin should ignore but doesn't. I'll also cook up a patch for gamin to to actually check for excluded paths in server/gamin_poll_basic.c. How does that sound?
Sounds good to me, will commit the patch tonight.
(In reply to comment #17) > Sounds good to me, will commit the patch tonight. Damn, I forgot about that (guess I'm too tired already) and committed it for you. Hope you don't mind, otherwise you can of course revert it and do it yourself (in case you've made some more changes).
Already told you to commit it last night ;-). Anyway, for the record: patch v5 without the file ref_count hack committed in revision 28917.
Ok, here's a follow up problem I think. With these changes committed, Thunar now crashes when toggling the visibility of hidden files: Thunar-CRITICAL **: thunar_tree_view_visible_func: assertion `(((__extension__ ({ GTypeInstance *__inst = (GTypeInstance*) ((file)); GType __t = ((thunar_file_get_type ())); gboolean __r; if (__inst && __inst->g_class && __inst->g_class->g_type == __t) __r = (!(0)); else __r = g_type_check_instance_is_a (__inst, __t); __r; }))))' failed aborting... Program received signal SIGABRT, Aborted. [Switching to Thread 0xb745e910 (LWP 6022)] 0xb7f58424 in __kernel_vsyscall () (gdb) bt #0 0xb7f58424 in __kernel_vsyscall () #1 0xb7704b77 in raise () from /lib/libc.so.6 #2 0xb77065b1 in abort () from /lib/libc.so.6 #3 0xb7851411 in g_logv () from /usr/lib/libglib-2.0.so.0 #4 0xb7851449 in g_log () from /usr/lib/libglib-2.0.so.0 #5 0xb785168c in g_return_if_fail_warning () from /usr/lib/libglib-2.0.so.0 #6 0x080bde93 in thunar_tree_view_visible_func (model=0x94c5cf0, file=0xb78de258, user_data=0x94d0150) at thunar-tree-view.c:1804 #7 0x080ba663 in thunar_tree_model_node_traverse_visible (node=0x9311660, user_data=0x94c5cf0) at thunar-tree-model.c:1765 #8 0xb7852490 in g_node_traverse_post_order () from /usr/lib/libglib-2.0.so.0 #9 0xb7852465 in g_node_traverse_post_order () from /usr/lib/libglib-2.0.so.0 #10 0xb7852e80 in g_node_traverse () from /usr/lib/libglib-2.0.so.0 #11 0x080ba14f in thunar_tree_model_refilter (model=0x94c5cf0) at thunar-tree-model.c:1964 #12 0x080bd7b9 in thunar_tree_view_set_show_hidden (view=0x94d0150, show_hidden=6022) at thunar-tree-view.c:2135 #13 0xb78efaf3 in g_object_set_property () from /usr/lib/libgobject-2.0.so.0 #14 0xb7edfeb7 in exo_bind_properties_transfer (src_object=0x94afd68, src_pspec=0x94ba980, dst_object=0x94d0150, dst_pspec=0x94c5c90, transform=0xb790b2d0 <g_value_transform>, user_data=0x0) at exo-binding.c:63 #15 0xb7edff3b in exo_bind_properties_notify (src_object=0x94afd68, src_pspec=0x94ba980, data=0x94cee68) at exo-binding.c:82 #16 0xb78f512a in g_cclosure_marshal_VOID__PARAM () from /usr/lib/libgobject-2.0.so.0 #17 0xb78e8779 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #18 0xb78fce3b in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #19 0xb78feacf in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #20 0xb78fee19 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #21 0xb78ecc91 in g_object_dispatch_properties_changed () from /usr/lib/libgobject-2.0.so.0 #22 0xb78e94df in g_object_notify_dispatcher () from /usr/lib/libgobject-2.0.so.0 #23 0xb78efb9f in g_object_set_property () from /usr/lib/libgobject-2.0.so.0 #24 0xb7edfeb7 in exo_bind_properties_transfer (src_object=0x92eb078, src_pspec=0x92e7980, dst_object=0x94afd68, dst_pspec=0x94ba980, transform=0xb790b2d0 <g_value_transform>, user_data=0x0) at exo-binding.c:63 #25 0xb7edff3b in exo_bind_properties_notify (src_object=0x92eb078, src_pspec=0x92e7980, data=0x94cf068) at exo-binding.c:82 #26 0xb78f512a in g_cclosure_marshal_VOID__PARAM () from /usr/lib/libgobject-2.0.so.0 #27 0xb78e8779 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #28 0xb78fce3b in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #29 0xb78feacf in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #30 0xb78fee19 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #31 0xb78ecc91 in g_object_dispatch_properties_changed () from /usr/lib/libgobject-2.0.so.0 #32 0xb78e94df in g_object_notify_dispatcher () from /usr/lib/libgobject-2.0.so.0 #33 0xb78ed729 in g_object_notify () from /usr/lib/libgobject-2.0.so.0 ---Type <return> to continue, or q <return> to quit--- #34 0xb78f5a0f in g_cclosure_marshal_VOID__VOID () from /usr/lib/libgobject-2.0.so.0 #35 0xb78e8779 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #36 0xb78fce3b in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #37 0xb78feacf in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #38 0xb78fee19 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #39 0xb7c363d5 in _gtk_action_emit_activate () from /usr/lib/libgtk-x11-2.0.so.0 #40 0xb7c37734 in closure_accel_activate () from /usr/lib/libgtk-x11-2.0.so.0 #41 0xb78e8779 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #42 0xb78fce3b in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #43 0xb78fe7f7 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #44 0xb78fee19 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #45 0xb7c32331 in gtk_accel_group_activate () from /usr/lib/libgtk-x11-2.0.so.0 #46 0xb7c32dfc in gtk_accel_groups_activate () from /usr/lib/libgtk-x11-2.0.so.0 #47 0xb7e045c2 in gtk_window_activate_key () from /usr/lib/libgtk-x11-2.0.so.0 #48 0xb7e0465c in gtk_window_key_press_event () from /usr/lib/libgtk-x11-2.0.so.0 #49 0xb7d03644 in _gtk_marshal_BOOLEAN__BOXED () from /usr/lib/libgtk-x11-2.0.so.0 #50 0xb78e7099 in g_type_class_meta_marshal () from /usr/lib/libgobject-2.0.so.0 #51 0xb78e8779 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 #52 0xb78fcfca in signal_emit_unlocked_R () from /usr/lib/libgobject-2.0.so.0 #53 0xb78fe7f7 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 #54 0xb78fee19 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 #55 0xb7df7777 in gtk_widget_event_internal () from /usr/lib/libgtk-x11-2.0.so.0 #56 0xb7cfdde9 in gtk_propagate_event () from /usr/lib/libgtk-x11-2.0.so.0 #57 0xb7cfee01 in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 #58 0xb7b9da9a in gdk_event_dispatch () from /usr/lib/libgdk-x11-2.0.so.0 #59 0xb7848116 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0 #60 0xb784b4c3 in g_main_context_iterate () from /usr/lib/libglib-2.0.so.0 #61 0xb784b8a7 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0 #62 0xb7cff2b4 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #63 0x080602ab in main (argc=153770920, argv=0x1) at main.c:243
Can you reproduce this? And how?
(In reply to comment #21) > Can you reproduce this? And how? Yes, I can, all the time. Open Thunar and press Ctrl+H while the tree view pane is shown. That's all it needs here.
Ouch, that's nasty. Can reproduce it though. Revision 28927 adds some extra debug checks can you bt with that?
(In reply to comment #23) > Ouch, that's nasty. Can reproduce it though. Revision 28927 adds some extra > debug checks can you bt with that? Hm, now I can't reproduce it anymore. Side note for the others: Support for excluding /proc and /dev is committed to trunk now, so these folders will no longer make Thunar freak out, no matter how FAM/gamin handles them.