fix: don't block UI when canceling lint process

[stevearc]

I've been getting mysterious nondeterministic freezes in Neovim for months now, but couldn't get a solid repro. I finally found a repro and tracked it down to LintProc:cancel(). In #522 we added some logic to cancel a process by killing it with sigint, waiting, then if it's still running kill it again with sigkill. The problem is that the delay is done with vim.wait(), which will block the UI thread. If you have a poorly-behaved linter (and I do, unfortunately it's for work and not something I can reasonably fix) that doesn't respond at all to sigint, the UI thread will be blocked for 10 seconds each time it fails to cancel.

I've replaced the synchronous vim.wait with a call to vim.defer_fn. From comments on the previous PR I gather that there is some concern about orphaned lint processes when Neovim exits. There's not much we can do if it crashes, but for normal operation I added a VimLeavePre hook that will cancel all running linters and block until they are killed.

Test Plan

First I found a repro without this patch

1. Open file
2. Call lint.try_lint() twice
3. Observe 10 second UI freeze

Then after applying this patch

1. Open file
2. Call lint.try_lint() twice
3. Observe no freeze
4. Run ps aux | grep <formatter> and verify that there are two processes running
5. Wait 10 seconds
6. Observe that the canceled process (the first one) was killed

And to check the exit behavior

1. Open file
2. Call lint.try_lint()
3. :q
4. Run ps aux | grep <formatter> and verify that the formatter is running
5. Observe 10 second UI freeze
6. Observe that Neovim quits and ps aux | grep <formatter> shows that the formatter process has been killed

[mfussenegger]

The entries in the running_procs_by_buf table get replaced after a :cancel call, so I think the VimLeavePre logic wouldn't work like this because the table won't contain the cancelled LintProcs anymore.

But the bigger problem is that if the cancel is deferred, you end up running multiple linters in parallel and you might get results updated/replaced differently.

Not sure how to deal with this. Maybe other options could be:

• Wrap your misbehaving linter in a small script that forwards sigint as sigkill
• try_lint could have a difference debounce mode, where instead of cancelling a proc, it lets it finish instead of starting a new process. Although this means the results would be stale
• New lint process gets chained onto the previous lint process, so it only starts once the previous one got cancelled - but this is probably more complex than I'd like.
• Allow to define per linter what signal to use - to allow killing it immediately without sigint. Or a custom cancel function per linter.

[stevearc]

The entries in the running_procs_by_buf table get replaced after a :cancel call, so I think the VimLeavePre logic wouldn't work like this because the table won't contain the cancelled LintProcs anymore.

That should be fine, right? Because we only need to cancel the currently running tasks on exit. Unless you're worried about the case of

1. Run lint process
2. Manually cancel lint process
3. Exit vim immediately
4. Vim exits before the sigkill timeout is reached, so process is orphaned

If that's what you're concerned about, I can fix it pretty easily by just having another way to keep track of all active LintProcs. LMK if this is what you were thinking of or if I'm misinterpreting.

But the bigger problem is that if the cancel is deferred, you end up running multiple linters in parallel and you might get results updated/replaced differently.

I don't think this can happen because when a LintProc is cancelled it won't publish the (now stale) diagnostics.

nvim-lint/lua/lint.lua

Lines 242 to 244 in ab83154

	if api.nvim_buf_is_valid(self.bufnr) and not self.cancelled then
	vim.diagnostic.set(self.ns, self.bufnr, diagnostics)
	end

[mfussenegger]

That should be fine, right? Because we only need to cancel the currently running tasks on exit. Unless you're worried about the case of

Hm, then I wonder if it is needed at all as part of this PR. They weren't getting cancelled before

I don't think this can happen because when a LintProc is cancelled it won't publish the (now stale) diagnostics.

Good point

[stevearc]

Hm, then I wonder if it is needed at all as part of this PR. They weren't getting cancelled before

If we want to scope the PR to just fixing the UI freeze, then we could leave out the VimLeavePre behavior. If we think of this PR as more broadly handling linter processes that don't terminate on SIGINT, then this is still useful for preventing orphaned processes when vim exits. I think this handles an important edge case, but if you think it adds too much complexity I can remove it from this PR.