Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: File-select dialog can open when going to exit #4582

Closed
wants to merge 4 commits into from

Conversation

ichizok
Copy link
Contributor

@ichizok ichizok commented Jun 24, 2019

Environment

gVim v8.1.1587 (with GTK3) on Ubuntu 18.04

Repro steps

$ gvim --clean
:set nobuflisted
:terminal

Click GUI window-close button and click "Yes" button on popup "Kill job in ...", then file-select dialog can open (or gVim finishes normally. Please try several times).
When click "Cancel", the dialog closes but opens again promptly.

Cause

Stacktrace when file-select dialog is shown:

(gdb) bt
#0  0x00007fbb55debbf9 in __GI___poll (fds=0x5612fa8a0400, nfds=4, timeout=19989) at ../sysdeps/unix/sysv/linux/poll.c:29
#1  0x00007fbb574b64c9 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007fbb574b6862 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007fbb58ba2db3 in gtk_dialog_run () at /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
#4  0x00005612f9a709cc in gui_mch_browse (saving=0, title=0x5612f9ac5723 "Edit File", dflt=0x0, ext=0x0, initdir=0x0, filter=0x5612f9ae7412 "") at gui_gtk.c:1271
#5  0x00005612f9abcca4 in do_browse (flags=0, title=0x5612f9ac5723 "Edit File", dflt=0x0, ext=0x0, initdir=0x0, filter=0x5612f9ae7388 "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=0x5612fa922000) at message.c:4074
#6  0x00005612f989debc in do_ecmd (fnum=0, ffname=0x0, sfname=0x0, eap=0x0, newlnum=1, flags=0, oldwin=0x5612fa8e3000) at ex_cmds.c:3836
#7  0x00005612f983fd65 in empty_curbuf (close_others=1, forceit=0, action=4) at buffer.c:1231
#8  0x00005612f98401be in do_buffer (action=4, start=1, dir=1, count=2, forceit=0) at buffer.c:1413
#9  0x00005612f983fb19 in do_bufdel (command=4, arg=0x5612f9addbf8 "", addr_count=1, start_bnr=2, end_bnr=2, forceit=0) at buffer.c:1173
#10 0x00005612f9a38896 in term_after_channel_closed (term=0x5612fa8986d0) at terminal.c:3026
#11 0x00005612f9a38a67 in term_channel_closed (ch=0x5612fa8f7400) at terminal.c:3090
#12 0x00005612f9a9a3a2 in channel_close (channel=0x5612fa8f7400, invoke_close_cb=1) at channel.c:3056
#13 0x00005612f9a9ab76 in channel_close_now (channel=0x5612fa8f7400) at channel.c:3378
#14 0x00005612f9a9c87f in channel_parse_messages () at channel.c:4418
#15 0x00005612f99349ed in parse_queued_messages () at misc2.c:4470
#16 0x00005612f9a3587f in term_try_stop_job (buf=0x5612fa922000) at terminal.c:1459
#17 0x00005612f98a8074 in check_changed_any (hidden=0, unload=0) at ex_cmds2.c:1340
#18 0x00005612f9a68188 in gui_shell_closed () at gui.c:860
#19 0x00005612f9a76ec2 in delete_event_cb (widget=0x5612fa6f4270, event=0x5612fa72ed40, data=0x0) at gui_gtk_x11.c:2941
...

In term_try_stop_job(), job->jv_status >= JOB_END cannot hold necessarily just after job_end() called (except Windows gVim).
https://github.com/vim/vim/blob/master/src/terminal.c#L1451-L1459

Therefore, do_browse() is called as above trace, file-select dialog opens.
When click "Cancel", do_browse() returns FAIL, so delete-buffer doesn't complete and first_term remains then the loop in term_channel_closed() isn't finished.
https://github.com/vim/vim/blob/master/src/terminal.c#L3064-L3093

Solution proposal

  • Don't open file-select dialog when going to exit
  • Check term->tl_channel_closed in term_channel_closed()

In addition

https://github.com/vim/vim/blob/master/src/ex_cmds.c#L3820-L3828

#ifdef FEAT_BROWSE
        if (cmdmod.browse)
        {
            if (
# ifdef FEAT_GUI
                !gui.in_use &&
# endif
                    au_has_group((char_u *)"FileExplorer"))
            {

It appears this FEAT_GUI isn't need since FEAT_BROWSE includes FEAT_GUI.

@codecov-io
Copy link

Codecov Report

Merging #4582 into master will increase coverage by <.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4582      +/-   ##
==========================================
+ Coverage   80.83%   80.84%   +<.01%     
==========================================
  Files         111      111              
  Lines      144576   144572       -4     
==========================================
+ Hits       116875   116876       +1     
+ Misses      27701    27696       -5
Impacted Files Coverage Δ
src/gui.c 57.95% <ø> (+0.05%) ⬆️
src/terminal.c 81.05% <100%> (-0.06%) ⬇️
src/ex_cmds.c 82.23% <50%> (+0.04%) ⬆️
src/gui_beval.c 62.5% <0%> (-0.44%) ⬇️
src/message.c 76.84% <0%> (-0.05%) ⬇️
src/window.c 87.18% <0%> (+0.03%) ⬆️
src/term.c 78.84% <0%> (+0.05%) ⬆️
src/ex_cmds2.c 88.17% <0%> (+0.11%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b044ff...5ea3158. Read the comment docs.

@brammool brammool closed this in 5c381eb Jun 25, 2019
@brammool
Copy link
Contributor

brammool commented Jun 25, 2019 via email

@ichizok ichizok deleted the fix/gui-bufdel branch June 25, 2019 05:00
@ichizok
Copy link
Contributor Author

ichizok commented Jun 25, 2019

@brammool
Thank you, but v8.1.1592 seems wrong (or, at least, not enough).
Although v8.1.1592 changes do_write(), need change in do_ecmd() to fix this issue.

brammool added a commit that referenced this pull request Jun 25, 2019
Problem:    May still start file dialog while exiting.
Solution:   Ignore the "browse" modifier in another place when exiiting.
            (Ozaki Kiichi, closes #4582)
@brammool
Copy link
Contributor

brammool commented Jun 25, 2019 via email

manuelschiller pushed a commit to manuelschiller/vim that referenced this pull request Nov 10, 2019
Problem:    May start file dialog while exiting.
Solution:   Ignore the "browse" modifier when exiting. (Ozaki Kiichi,
            closes vim#4582
manuelschiller pushed a commit to manuelschiller/vim that referenced this pull request Nov 10, 2019
Problem:    May still start file dialog while exiting.
Solution:   Ignore the "browse" modifier in another place when exiiting.
            (Ozaki Kiichi, closes vim#4582)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants