vim-gutentags icon indicating copy to clipboard operation
vim-gutentags copied to clipboard

Gutentags makes neovim to hang when quitting while tags are being generated

Open dpobel opened this issue 6 years ago • 37 comments

The title says it all. If the tags are being generated, when quitting neovim, it hangs for a few seconds.

I think this was not the case before using the Job API.

dpobel avatar Apr 09 '18 20:04 dpobel

Can confirm the same issue

  • NVIM v0.2.2
  • Gutentags master (commit 327bd97)

It does not happen on v1.0.0

justinhoward avatar Apr 12 '18 23:04 justinhoward

It's not a "hang", Nvim is waiting for the process to finish gracefully before SIGKILLing it.

Looks like the script handles SIGTERM.

https://github.com/ludovicchabant/vim-gutentags/blob/master/plat/unix/update_tags.sh#L95

Hmm..

justinmk avatar Apr 12 '18 23:04 justinmk

@justinmk Fair enough. I really appreciate gutentags. It's better than any other solution I've tried for vim. It totally makes sense that you wouldn't want to just SIGKILL ctags.

However, that's typically not the behavior I expect from vim. It's especially inconvenient when opening vim to do something quick like writing a git commit message. For some modest size projects, I need to wait 5 seconds or so for ctags to finish up. Are there alternatives to a SIGKILL? I'm out of my depth here with the Job API, but can a process be detached on exit?

justinhoward avatar Apr 13 '18 00:04 justinhoward

Yes, gutentags can start the process with "detach" set, like this:

let a:opts['detach'] = 1
jobstart(a:cmd, a:opts)

But that means the script needs to kill itself at some point. @justinhoward Does that change fix the issue for you?

justinmk avatar Apr 13 '18 00:04 justinmk

This can be handled more gracefully by using an atomic replace, i.e. move the existing tags file to a cache then start ctags. If it completes successfully, delete the moved tags file as it is no longer needed. If neovim shutdown or suspend is encountered, kill -9 ctags_pid && mv tags.old tags. It's OK to kill ctags as soon as we know a suspend/quit is there since its output is being discarded.

mqudsi avatar Apr 14 '18 20:04 mqudsi

The script is already doing what you're saying, I believe. See this line. It does everything against a "temp" tags file, and replaces it at the end. This is mainly because otherwise, while the job is running, Vim would show you incomplete or even empty symbols.

I'm actually thinking that killing the background job is indeed the correct solution. It might be a bit uglier on Windows, but it should otherwise work OK. Using the detach option would make it possible to close Vim while the script is running, and immediately re-open Vim and triggering a new instance of the script, thus resulting in 2 jobs fighting each other, so that's not awesome.

It looks like Vim supports an option that will send a signal to the job when Vim exits (stoponexit: SIGTERM), but Neovim doesn't seem to support that... so I think we would need an auto-command to cleanup the job under Neovim? Or am I missing something with Neovim?

ludovicchabant avatar Apr 17 '18 07:04 ludovicchabant

Oh, sorry, I should have checked the source first!

What about on suspend? If I ctrl+z there's a delay until neovim is backgrounded and I think it's due to the running job?

mqudsi avatar Apr 17 '18 16:04 mqudsi

This might be an Nvim issue, I did some testing.

justinmk avatar Apr 18 '18 06:04 justinmk

Yeah it's a Neovim issue only -- Vim8 defaults to detaching. I'm just suggesting that, instead of making Neovim also detach like Vim8, that we change the behaviour for both, and make them both kill the script process.

ludovicchabant avatar Apr 18 '18 16:04 ludovicchabant

@dpobel @justinhoward does the issue go away if you disable 'fsync' and swapfiles? E.g.:

nvim -i NONE -c "set nofsync"

Slow fsync() is the root cause of the issue I am seeing.

justinmk avatar Apr 18 '18 20:04 justinmk

@justinmk that could be the case as the problem is particularly noticeable on WSL which has some not-insignificant latency for I/O independent of actual device IO performance. I wonder if fdatasync(2) would would be any faster?

mqudsi avatar Apr 18 '18 23:04 mqudsi

@justinmk Disabling fsync doesn't seem to affect the delay after quit. My test procedure was:

$ time nvim -i NONE -c "set nofsync" -c "set noswapfile" -c ':q' path/to/file
nvim -i NONE -c "set nofsync" -c "set noswapfile" -c ':q'   0.23s user 0.11s system 14% cpu 2.290 total

$ time nvim -c ':q' path/to/file
nvim -c ':q' path/to/file  0.29s user 0.13s system 17% cpu 2.328 total

justinhoward avatar Apr 19 '18 18:04 justinhoward

@justinhoward Then I can't reproduce the issue.

You should:

  • try the prerelease version: https://github.com/neovim/neovim/releases/tag/nightly
  • check ~/.local/share/nvim/log to see if there's a "hang" message

justinmk avatar Apr 20 '18 08:04 justinmk

Looking back at this issue and Neovim's help, it looks like it's not really fixable at the moment? Someone more familiar with Neovim can correct me but:

  1. Looking at :help jobstart(), it says this about the detach option: "Detach the job process from the nvim process. The process will not get killed when nvim exits.". This indicates that Neovim does indeed try to kill the script if you exit while the tags are being generated.

  2. Looking at :help jobstop(), it says: "Stop job-id by sending SIGTERM to the job process. If the process does not terminate after a timeout then SIGKILL will be sent."

So upon exit, it seems that Neovim tries to send SIGTERM, and after a timeout, SIGKILL. This "hang" that people are experiencing on Neovim seems to be that timeout between SIGTERM and SIGKILL. I didn't find anything to be able to send SIGKILL directly, or set the timeout... so it looks like it's not possible to fix at the moment?

We could add an option to use detach, which would remove the hang, but could lead to other problems as I mentioned before.... any other ideas?

ludovicchabant avatar May 09 '18 04:05 ludovicchabant

This indicates that Neovim does indeed try to kill the script if you exit while the tags are being generated.

Yes, if detach is not set.

This "hang" that people are experiencing on Neovim seems to be that timeout between SIGTERM and SIGKILL

That's what I said originally but I couldn't reproduce it and found the fsync behavior instead.

For those using Nvim HEAD (0.3) and :verb set fsync? reports nofsync, and you still see this issue, can you try deleting this line and see if it makes any difference?

I didn't find anything to be able to send SIGKILL directly, or set the timeout...

If you sigkill on exit it's going to leave the .lock file ...

I'm not convinced the SIGKILL timer is the issue. Waiting on answers to my above questions.

justinmk avatar May 09 '18 07:05 justinmk

I don't think it's a SIGKILL timer issue, because ctags -R . &; sudo killall -SIGTERM ctags stops immediately.

--

Nevermind, I need to start over. With set nofsync and neoinclude disabled I have no issues. (neoinclude's behavior changes based off the presence of a ctags file.)

mqudsi avatar May 11 '18 20:05 mqudsi

@justinmk just tried your suggestion. Verified nvim reported back "nofsync" and commented out the line in question. I'm no longer able to reproduce the long hang on my local machine using both nvim HEAD and 0.2.2.

dkalowsk avatar May 23 '18 18:05 dkalowsk

Okay thanks for the investigations, everybody! I'm just no quite sure what to do with all this so suggestions are welcome :)

Doing some simple tests on my machine with a big enough repo, I can see that:

  • Adding stoponexit: term to Vim8's job options correctly terminates the background job on exit. I can start Vim, see that the update_tags.sh script is running, quit Vim, and it gets promptly shut down.

  • On Neovim (without changing the current code since Neovim supposedly terminates jobs on exit by default), it doesn't seem to work. I run Neovim, see that the script is running, quit Neovim, and it doesn't go back to the shell, as it waits for the job to terminate. I have to manually kill it to get Neovim to finish exiting.

  • Still with Neovim, if I comment out the trap line in update_tags.sh, Neovim does seem like it exits right away now (fixing the issue mentioned in the previous point), but it actually only kills the script! The ctags process (spawned from the script) is still running!

I don't know why the ctags process doesn't get killed along with the script in that case, but I suppose Neovim doesn't do a "parent and children" termination, and instead just kills the one PID... does anybody with a better understanding of all this knows if there's a way for either (1) Neovim to kill the whole process hierarchy or (2) for update_tags.sh to somehow trap its own termination so it can kill its children processes?

ludovicchabant avatar Jun 09 '18 17:06 ludovicchabant

@ludovicchabant Really need to know the exact version of Nvim you tried.

I don't know why the ctags process doesn't get killed along with the script in that case,

Sounds like the signal was not sent to the process group. This was fixed on Nvim HEAD.

Neovim doesn't do a "parent and children" termination

This was fixed on Nvim HEAD. It's a bug in Nvim 0.2.2 and older.

justinmk avatar Jun 10 '18 09:06 justinmk

Ah yeah thanks for the info, I'm on Nvim 0.2.2, so it looks like we would have to wait for the next release... at which point it might just... fix everything?

And alternatively, we would add "stoponexit: term" to the Vim8 job options so both flavours of Vim kill the job on exit?

ludovicchabant avatar Jun 11 '18 05:06 ludovicchabant

This bug still exists in neovim 0.3.0, since termination should work correctly now, has it been implemented for neovim?

xiamaz avatar Jun 27 '18 16:06 xiamaz

I'm on Neovim v0.3.1 and I'm still seeing this issue.

timnolte avatar Oct 07 '18 19:10 timnolte

@timnolte Do you have :set fsync? set or not? (Review the thread above...)

justinmk avatar Oct 07 '18 20:10 justinmk

I just tried Neovim 0.3.1 and unlike the last time I tried (see my message from June 9th), this time it works as expected (i.e. the ctags process gets killed when I exit Neovim), so I guess the Neovim bug was fixed.

By the way, in case it wasn't clear, the Vim8 code path now specifically kills the background job on exit (see this line).

ludovicchabant avatar Nov 12 '18 08:11 ludovicchabant

I'm on NVIM v0.3.2-962-g231de7253 and vim-gutentags 93616e4c0ccfafe52ae329c7dd220d7b5c7d5f80 (latest master at the time of writing) and I still see this issue. set fsync? reports nofsync.

Any ideas? I can help test things if necessary.

ruipin avatar Mar 01 '19 13:03 ruipin

As reported in #168, I have Neovim v0.3.4 now and I can't reproduce the issue. Are there more specific repro steps than "quit Neovim while tags are generating"?

(it might also be a platform thing... I'm testing this on macOS)

ludovicchabant avatar Mar 13 '19 03:03 ludovicchabant

According to https://github.com/ludovicchabant/vim-gutentags/issues/249#issuecomment-533951319 this might have improved in Neovim 0.4, where it will now exit more quickly (via SIGPIPE). However, it would still cause a message then, which should be fixed with https://github.com/ludovicchabant/vim-gutentags/pull/251.

blueyed avatar Oct 07 '19 12:10 blueyed

Just checking in that I still get some 2-10 second hangs after closing Neovim 0.4.2 on MacOS with latest version of vim-gutentags. And set fsync? reports back fsync.

phantomwhale avatar Dec 12 '19 04:12 phantomwhale

And set fsync? reports back fsync.

So disable it, as suggested above.

justinmk avatar Dec 12 '19 07:12 justinmk

Ah, whoops, did add set nofsync and things seemed OK...

...but just moved to a new MacBook (so picked up latest versions of everything) and ran vim against some of my dotfiles (not a big repository by any means) and it kicked off a ctags process that didn't die when I shut vim (and stayed running after I hit Ctrl-c).

I've still got nofsync in my vimrc, but ctags doesn't die. Removing the plugin confirms it's gutentags doing this (not some rouge git hook I'd forgotten about or something)

[RE-EDIT] still getting vim hangs, and the occasional ctags process (churning at 100% CPU) that never dies off

phantomwhale avatar Dec 20 '19 10:12 phantomwhale