[patch] GtkFileChooser and gvim's odd use of gtk_main()
Tim Starling
1 every time an event is received, causing GTK to write to the disk
excessively.
gtk_main() has always stored the clipboard on the same trigger, so it's
fair to say that exiting gtk_main() constantly is not the way the GTK
developers expected an application to work. People who know stuff about
GTK agree:
http://mail.gnome.org/archives/gtk-list/2008-July/msg00131.html
My interest in this started when I discovered that GtkFileChooser had
been disabled. I missed it:
https://bugs.launchpad.net/ubuntu/+source/vim/+bug/365860
It's been 7 months now since that plaintive cry, so I gave up waiting
and wrote a patch. This is my first GTK project but it looks pretty
straightforward. I read the source code for gtk_main() and some other
things.
Emmanuele Bassi's advice was to create a separate GMainLoop:
http://mail.gnome.org/archives/gtk-list/2008-July/msg00146.html
That turns out to be unnecessary, since all GMainLoop does is calls
g_main_context_iteration() repeatedly, until it's time to exit. The
actual task of registering event sources and calling select() is done by
GMainContext, which exists whether or not a GMainLoop has been created.
Thus by calling g_main_context_iteration() at the relevant places, vim
can leave main_loop() unmodified.
My patch does this, and removes all calls to gtk_main_quit() except the
ones in modal dialogs. Modal dialogs are now implemented by calling
gtk_main() once only, and allowing the button handlers to call
gtk_main_quit(), just like in a regular GTK application.
We don't seem to be missing anything substantial by never calling
gtk_main(). You can register callbacks to be called when gtk_main()
exits, but luckily vim isn't doing that or else they'd be called on the
first keypress and then never again. Of course we miss out on that final
sync of recently-used.xbel, but it seems that it's already synced every
time it's changed, via the "changed" signal (see
gtk_recent_manager_changed() and its callers in gtk/gtkrecentmanager.c).
I haven't done cross-version tests on this patch, but
g_main_context_iteration() exists back to at least GLib 1.3.9 (September
2001).
Attached and online at:
http://tstarling.com/stuff/fix-gtk-main.patch
-- Tim Starling
Bram Moolenaar
Tim Starling wrote:
I'm very glad to see someone is picking up a GTK problem. Hardly any
work was done on GTK for a long time.
I would appreciate it if a few people try this out and report any
problems they notice.
--
I used to wonder about the meaning of life. But I looked it
up in the dictionary under "L" and there it was - the meaning
of life. It was less than I expected. - Dogbert
/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
James Vega
The bug Tim fixed was something I had been planning on taking a look at, so
I'll definitely try out the patch soon and report back.
Thanks for the patch, Tim!
--
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jame...@debian.org>
James Vega
> On Fri, Nov 13, 2009 at 10:39:29PM +0100, Bram Moolenaar wrote:
>> Tim Starling wrote:
>> > Thus by calling g_main_context_iteration() at the relevant places, vim
>> > can leave main_loop() unmodified.
>> >
>> > My patch does this, and removes all calls to gtk_main_quit() except the
>> > ones in modal dialogs. Modal dialogs are now implemented by calling
>> > gtk_main() once only, and allowing the button handlers to call
>> > gtk_main_quit(), just like in a regular GTK application.
>> > Of course we miss out on that final
>> > sync of recently-used.xbel, but it seems that it's already synced every
>> > time it's changed, via the "changed" signal (see
>> > gtk_recent_manager_changed() and its callers in gtk/gtkrecentmanager.c).
>>
>> work was done on GTK for a long time.
>>
>> I would appreciate it if a few people try this out and report any
>> problems they notice.
>
> The bug Tim fixed was something I had been planning on taking a look at, so
> I'll definitely try out the patch soon and report back.
I've given it a shot and it's working well for me. One thing I did
notice is that the files Vim edits aren't added to Gtk's recently used
list. Based on the patch, this doesn't look like it's a regression in
behavior from the last time Vim was using GtkFileChooser.
I think it'd be good to get the patch as is included so Vim is no longer
using a deprecated (and as of Gtk3 removed) widget. Also,
GtkFileChooser is more intuitive to use than GtkFileSelection (where
it's non-obvious how to select hidden files).
It would be interesting to look into how (and whether) Vim should make
use of Gtk's recently used list, especially since not all of Vim's
buffers directly relate to an actual file.
--
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jame...@jamessan.com>
Tim Starling
> I've given it a shot and it's working well for me. One thing I did
> notice is that the files Vim edits aren't added to Gtk's recently used
> list. Based on the patch, this doesn't look like it's a regression in
> behavior from the last time Vim was using GtkFileChooser.
>
> I think it'd be good to get the patch as is included so Vim is no longer
> using a deprecated (and as of Gtk3 removed) widget. Also,
> GtkFileChooser is more intuitive to use than GtkFileSelection (where
> it's non-obvious how to select hidden files).
>
> It would be interesting to look into how (and whether) Vim should make
> use of Gtk's recently used list, especially since not all of Vim's
> buffers directly relate to an actual file.
>
>
I imagine it would be done in a manner along the lines of the attached
patch file. It was briefly tested.
I'm not a big fan of #ifdef so I added a function called
gui_mch_file_opened() which allows any GUI module to perform tasks on
file open. I put it near similar #ifdef-based notifications for Sun
Workshop and Netbeans.
My knowledge of the vim source is rudimentary. Some more callers might
have to be added, such as when saving a new file.
-- Tim Starling
Bram Moolenaar
Tim Starling wrote:
It looks like this will also add files when using a command like
":grep" and Vim scripts looking through files. It's probably better to
only do this when the user typed the edit command.
# include <gio/gio.h>
Since when was this file available? Might need a configure check.
--
BLACK KNIGHT: None shall pass.
ARTHUR: I have no quarrel with you, brave Sir knight, but I must cross
this bridge.
BLACK KNIGHT: Then you shall die.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
James Vega
> Tim Starling wrote:
>> GtkFileChooser is disabled in gvim, because gvim exits gtk_main() level
>> 1 every time an event is received, causing GTK to write to the disk
>> excessively.
>>
>>
>> That turns out to be unnecessary, since all GMainLoop does is calls
>> g_main_context_iteration() repeatedly, until it's time to exit. The
>> actual task of registering event sources and calling select() is done by
>> GMainContext, which exists whether or not a GMainLoop has been created.
>> Thus by calling g_main_context_iteration() at the relevant places, vim
>> can leave main_loop() unmodified.
>>
>> My patch does this, and removes all calls to gtk_main_quit() except the
>> ones in modal dialogs. Modal dialogs are now implemented by calling
>> gtk_main() once only, and allowing the button handlers to call
>> gtk_main_quit(), just like in a regular GTK application.
>
> I'm very glad to see someone is picking up a GTK problem. Hardly any
> work was done on GTK for a long time.
>
> I would appreciate it if a few people try this out and report any
> problems they notice.
Given the recent removal of GTK 1 support, attached is an updated patch.
SungHyun Nam
Though I never use the file open dialog, I just tried the patch.
On the Ubuntu 9.04, file chooser worked fine.
But, on the cygwin, a problem occurs.
Without the patch, there's no problem to open file open dialog.
$ uname -a
CYGWIN_NT-5.1 namsh 1.7.5(0.225/5/3) 2010-04-12 19:07 i686 Cygwin
If I run 'vim -f -g' (foreground), file chooser worked fine.
If I run 'vim -g' (background), VIM died with SIGSEGV.
I run vim as below to get backtrace:
$ LANG= ./vim.exe -g -u NONE -U NONE --noplugin
$ gdb ./vim.exe
GNU gdb 6.8.0.20080328-cvs (cygwin-special)
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-cygwin"...
(gdb) attach 4036
Attaching to program `/s/vim-unix/src/vim.exe', process 4036
...
... [snip...]
...
(gdb) c
Continuing.
Program received signal SIGSEGV, Segmentation fault.
[Switching to thread 4036.0xa50]
0x76ba1ef4 in ?? ()
(gdb) bt
#0 0x76ba1ef4 in ?? ()
#1 0x6ff41755 in g_module_symbol () from /usr/bin/cyggmodule-2.0-0.dll
#2 0x6ff41db3 in g_module_open () from /usr/bin/cyggmodule-2.0-0.dll
#3 0x681675a5 in g_io_modules_load_all_in_directory ()
from /usr/bin/cyggio-2.0-0.dll
#4 0x6e5a3526 in g_type_module_use () from /usr/bin/cyggobject-2.0-0.dll
#5 0x68167348 in g_io_modules_load_all_in_directory ()
from /usr/bin/cyggio-2.0-0.dll
#6 0x68167466 in g_io_modules_load_all_in_directory ()
from /usr/bin/cyggio-2.0-0.dll
#7 0x6817dffb in g_vfs_is_active () from /usr/bin/cyggio-2.0-0.dll
#8 0x6aa8b50e in g_once_impl () from /usr/bin/cygglib-2.0-0.dll
#9 0x6817dc5c in g_vfs_get_default () from /usr/bin/cyggio-2.0-0.dll
#10 0x6814e773 in g_file_new_for_path () from /usr/bin/cyggio-2.0-0.dll
#11 0x6af6030f in gtk_recent_manager_get_for_screen ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#12 0x6e588f36 in g_object_freeze_notify () from
/usr/bin/cyggobject-2.0-0.dll
#13 0x6e58a4a6 in g_object_newv () from /usr/bin/cyggobject-2.0-0.dll
#14 0x6e58a9ed in g_object_new_valist () from /usr/bin/cyggobject-2.0-0.dll
#15 0x6e58ab46 in g_object_new () from /usr/bin/cyggobject-2.0-0.dll
#16 0x6af5fddb in gtk_recent_manager_new () from
/usr/bin/cyggtk-x11-2.0-0.dll
#17 0x6af5fdfc in gtk_recent_manager_get_default ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#18 0x6aebbef9 in gtk_file_chooser_button_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#19 0x6e5a256a in g_type_create_instance () from
/usr/bin/cyggobject-2.0-0.dll
#20 0x6e588e2a in g_object_freeze_notify () from
/usr/bin/cyggobject-2.0-0.dll
#21 0x6aebe945 in gtk_file_chooser_button_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#22 0x6e58a4a6 in g_object_newv () from /usr/bin/cyggobject-2.0-0.dll
#23 0x6e58a9ed in g_object_new_valist () from /usr/bin/cyggobject-2.0-0.dll
#24 0x6e58ab46 in g_object_new () from /usr/bin/cyggobject-2.0-0.dll
#25 0x6aeaf11b in gtk_file_chooser_button_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#26 0x6aec4513 in gtk_file_chooser_widget_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#27 0x6e58a4a6 in g_object_newv () from /usr/bin/cyggobject-2.0-0.dll
#28 0x6e58a9ed in g_object_new_valist () from /usr/bin/cyggobject-2.0-0.dll
---Type <return> to continue, or q <return> to quit---
#29 0x6e58ab46 in g_object_new () from /usr/bin/cyggobject-2.0-0.dll
#30 0x6aec01e5 in gtk_file_chooser_dialog_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#31 0x6e58a4a6 in g_object_newv () from /usr/bin/cyggobject-2.0-0.dll
#32 0x6e58aa18 in g_object_new_valist () from /usr/bin/cyggobject-2.0-0.dll
#33 0x6e58ab46 in g_object_new () from /usr/bin/cyggobject-2.0-0.dll
#34 0x6aebfde0 in gtk_file_chooser_dialog_get_type ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#35 0x6aebfe89 in gtk_file_chooser_dialog_new ()
from /usr/bin/cyggtk-x11-2.0-0.dll
#36 0x0054f73e in gui_mch_browse (saving=0, title=0x5785a5 "Edit File",
dflt=0xf05030 "", ext=0x0, initdir=0x0,
filter=0x584f5c "All Files (*)\t*\nC source (*.c,
*.h)\t*.c;*.h\nC++ source (*.cpp, *.hpp)\t*.cpp;*.hpp\nVim files (*.vim,
_vimrc, _gvimrc)\t*.vim;_vimrc;_gvimrc\n") at gui_gtk.c:859
#37 0x0049cdce in do_browse (flags=0, title=0x5785a5 "Edit File",
dflt=0xf05030 "", ext=0x0, initdir=0x0,
filter=0x584f5c "All Files (*)\t*\nC source (*.c,
*.h)\t*.c;*.h\nC++ source (*.cpp, *.hpp)\t*.cpp;*.hpp\nVim files (*.vim,
_vimrc, _gvimrc)\t*.vim;_vimrc;_gvimrc\n", buf=0xc6b150) at message.c:3804
#38 0x0043c745 in do_ecmd (fnum=0, ffname=0xf05030 "", sfname=0x0,
eap=0x22c9e0, newlnum=1, flags=0, oldwin=0xc6a1c8) at ex_cmds.c:3177
#39 0x00455571 in do_exedit (eap=0x22c9e0, old_curwin=0x0) at
ex_docmd.c:7626
#40 0x00455f09 in ex_edit (eap=0x22c9e0) at ex_docmd.c:7522
#41 0x004526d9 in do_cmdline (cmdline=0x0, getline=0x461c10 <getexline>,
cookie=0x0, flags=0) at ex_docmd.c:2640
#42 0x004c0cff in nv_colon (cap=0x22cb38) at normal.c:5245
#43 0x004c2758 in normal_cmd (oap=0x22cb9c, toplevel=1) at normal.c:1188
#44 0x0048441f in main_loop (cmdwin=0, noexmode=0) at main.c:1216
#45 0x0048719a in main (argc=2090061788, argv=0x612288b4) at main.c:960
James Vega
Thanks for trying the patch out! I'd like to see this get included, so
it's good to see people giving it a shot.
As for the crash, this happens without the patch as well, if the Gtk2
file chooser is enabled. This even happens in Vim 7.2 if the Gtk2 file
chooser is enabled, so I'm wondering if there isn't something else going
on here. This reminded me of a bug[0] I had seen in RedHat's tracker a
while back which silently died, but may be relevant.
[0]: https://bugzilla.redhat.com/show_bug.cgi?id=488652
James Vega
Even the original changeset that introduced the use of the file chooser
(a20218704019) crashes in cygwin. I've been able to get other
applications (eog) to build and run in cygwin and use the file chooser
there. So, it doesn't look like this is a problem specific to the
cygwin port of the Gtk libraries.
If need be, cygwin builds could use the old file chooser but that
doesn't help with the goal of removing old Gtk code. Remember, the old
file chooser is likely being removed for Gtk3 along with all the other
deprecated widgets.
In the 2+ years the file chooser was active, before being disabled in
7.2b.026, there weren't any reported bugs that I can recall about gvim
crashing when using the file chooser. I'm only subscribed to vim-dev,
though. At any rate, it seems that either the bug didn't exist back
then and Gtk/Glib has changed in such a way that Vim's use of it is now
wrong or no one encountered (due to not using the file chooser) or was
bothered enough to report the bug.