GNOME Bugzilla – Bug 172046
Muliple MenuTree changed notifications per file change
Last modified: 2010-10-06 13:51:49 UTC
Sometimes, two FAM event emissions with only a short period of time in between lead to a situation in which my MenuTreeChangedFunc is called over and over. I could reproduce this bug for a custom menu file I put under ~/.config/menus/applications-merged/gnome-menu-editor.menu with mv ~/.config/menus/applications-merged/gnome-menu-editor.menu \ ~/.config/menus/applications-merged/gnome-menu-editor-old.menu; \ sleep 0.5; mv ~/.config/menus/applications-merged/gnome-menu-editor-old.menu \ ~/.config/menus/applications-merged/gnome-menu-editor.menu
Hrm it doesn't seem to be really reproducible using the above command; it works at least always when creating a desktop file in ~/.local/share/applications and shortly after creating or modifying ~/.config/menus/applications-merged/gnome-menu-editor.menu. Can I provide any extra debug info? MENU_VERBOSE wasn't verbose enough. At least it didn't tell me why the second, third, nth callback was called; it just informed me that the first callback was called because of the addition of a custom desktop file.
Created attachment 39411 [details] Verbose gnome-menu-editor session I've attached a sample of a problematic gnome-menu-editor session with MENU_VERBOSE enabled. Detailed information is provided in the tarball's README file.
Looks like that thing is NOT called forever. The problem is that it is called for the CachedDir in question, foreach EntryDirectory, i.e. menu node. In entry-directories.c, each EntryDirectory registers a EntryDirectoryChangedFunc monitor and on CachedDir changes, each one is called. Therefore, if your menu tree has N menus, the callback is called N times.
This only seems to be relevant if the menu tree is instantly re-read after the change monitors have been notified, as it is the case with gnome-menu-editor.
Right, known issue - the monitoring stuff needs to be significantly re-worked. See bug #160194. Really, the way things are currently, the monitor callbacks should only queue an idle handler. That's what the panel does too.
Created attachment 142506 [details] [review] Compress GMenuTree monitor notifications Changes to a single file on disk could potentially cause changes to many nodes in the menu tree. Since we make no attempt to determine what actually changed versus potentially changed and, in any case, don't provide any fine-grained indication of what node changed, we need to compress our changes in order to avoid providing many identical notifications. Do this in the standard way by using an idle to defer the actual work of notification. The idle priorities for this and for the idle used in menu-monitor.c are adjusted to give maximum opportunity for compression.
I think that the original confusion here about the circumstances where this occurs probably has to do with the nature of the way that notifications occur. When notified of a change, GMenuTree deletes everything and that removes the monitors. And they aren't created again until the trees are refetched. So, as long as you don't *do* anything in your notification callbacks, they look like they are compressed. It's not a very useful form of compression, however. Other note is that the reason I did the compression in menu-layout.c rather than in gmenu-tree.c, is that menu-layout.c is where the duplicates are generated - where we take a notification on a leaf directory and turn it into an undifferentiated notification on the root directory
Created attachment 170924 [details] [review] Alternative patch Hrm, I hadn't see Owen's patch, so I wrote this today, which is quite similar (except that it also compresses notifications at the CachedDir level). I'll commit this after the freeze.