-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
LSP: _changetracking.lua:154: Invalid buffer id: (race condition in 'initialize' RPC?) #28575
Comments
Something like that. Possibly related: #27383 |
Assuming this is fixed by #28875 |
@justinmk I'm not sure if this is the same bug. |
This comment was marked as outdated.
This comment was marked as outdated.
You can consistently get a bad buffer into local dummy = vim.lsp.start_client({
name = "dummy",
cmd = { "dummylsp" },
on_init = function()
vim.api.nvim_buf_delete(0, {})
end
})
vim.api.nvim_create_autocmd("FileType", {
pattern = { "go" },
callback = function()
vim.lsp.buf_attach_client(0, dummy)
end
})
edit: using |
@sapient-cogbag can you test out #28914 ? |
Question: if + if not api.nvim_buf_is_loaded(bufnr) then
+ return
+ end
changetracking.init(self, bufnr) -- <=== Error comes from here. Note that further down there is some kind of bufnr validation
if not vim.tbl_get(self.server_capabilities, 'textDocumentSync', 'openClose') then
return
end
- if not api.nvim_buf_is_loaded(bufnr) then
- return
- end
local filetype = vim.bo[bufnr].filetype or having such a logic inside |
@sapient-cogbag please report back if the issue persist after applying #28914 |
Sorry for the delay in reply, I appreciate the effort that has gone into solving this. I just installed the latest neovim When I examine local hover_opts = {
close_events = { "InsertLeavePre", "InsertEnter" },
focus = false,
border = "rounded",
max_height = 20
} Somehow when the buffers are deleted they aren't getting detached from the language server when they're destroyed, which means that the check for I see: #28913, but I don't think it was merged to Noticing that the buffer was not successfully detached, I also looked at the things I call on If a language server then tries to send an initialize response (either the initial one or some kind of repeat) after some sort of failed detach, then that could still replicate the issue, especially if the failure to detach also means that the language server process isn't auto-stopped by someone's config when nothing is attached. My config does not auto-stop any language servers though, unless I reload my config or manually stop them with my version of Either that, or buffers that get deleted in certain ways don't invoke the detach machinery and, if a language server responds to |
Also, I explicitly (myself) add an autocmd to all buffers that manually detaches buffers from clients when they are deleted (which actually doesn't check for buffer validity). This may also be related. The relevant code is as follows: local augid = vim.api.nvim_create_augroup("lsp-client-detach-on-buffer-deletion", { clear = true })
vim.api.nvim_create_autocmd({ "LspAttach" }, {
group = augid,
desc = "Add autocmds to attached buffers so their deletion detaches them from the LSP client",
callback = function(event)
local bufnr = event.buf
local client = vim.lsp.get_client_by_id(event.data.client_id)
if client ~= nil then
-- Create the buffer-local autocmd
--
-- `BufUnload` is not something ok for this as unloading seems to be quite common and will result in weird duplication.
vim.api.nvim_create_autocmd({ "BufDelete" }, {
group = augid,
buffer = bufnr,
desc = "Detach this buffer from LSP client - [" .. client.name .. "] - when the buffer is deleted",
callback = function(buflocal_event)
-- only detach if the buffer is actually still attached to the client.
if client.attached_buffers[bufnr] then
-- If the buffer is invalid, buf_detach_client won't work, so we just delete it from the array
-- directly.
--[[if not vim.api.nvim_buf_is_valid(bufnr) then
client.attached_buffers[bufnr] = nil
else]]--
vim.lsp.buf_detach_client(bufnr, client.id)
-- end
end
end
})
end
end
}) Edit: |
Looking inside Moreover, this also applies to the functions that call those autocmds, which also manage the attached buffer information. The In particular, note the following:
The sequence of events that triggers this issue - if my hypothesis is correct - is then as follows:
Hopefully this is helpful, as I suspect the real issue is now related to buffer deletion and the |
@sapient-cogbag I cannot reproduce the error using your |
Yes, the I have a better understanding of the actual issue now (documented above is my new hypothesis) after the recent commit, but I still don't know exactly what could cause the buffer detach-on-delete machinery to fail. I can dig around some more in the |
Until I know the root cause of the failure to remove from the |
How consistently can you reproduce this using your config? |
My own config produces this constantly (I have to turn my autohover/auto-documentation off to even be able to type something without 20 different errors getting in the way). I can work somewhat without the autohover but it also pops up with manual hover too, so every time I look at docs. This is related to the fact that my markdown LS is attaching to the hover popups, but it works with other LSes as well e.g. sometimes it happens when using fzf-lua and preview mode (and having language server stuff there turns out to be surprisingly useful because I can scan through documents and see issues quickly), and it also sonetimes happens when I use the config reload mechanism. I'll attempt a new For this I'll see about creating an LspDetach callback that errors, and some other things. |
@sapient-cogbag if you share your config + some setup/repro steps I can take a look. |
My config is here. I will warn you it's an absolute disaster though because it was made in a massive rush, and then I piled hacks on top of other hacks to fix issues but now it's... really something :/, honestly it's a bit embarrassing xP. Make sure to stay with the The main thing is that it still uses the old Packer plugin repo, and it should automatically bootstrap on that when you first load it, then you'll want to run It also produces a lot of log messages (based on it's discovery of various natively installed language servers). The main thing here is that it won't do the standard The most reliable way to trigger the issue is:
On another note, I put logging inside the runtime itself while I've been trying to create a minimal reproduction, and it seems like with my config enabled, One problem I have with creating a minimal reproduction is that my config is multi-file, so instead of trying to cut out bits of config and localise the issue, I'm stuck trying to rebuild the actual causative scenario from scratch. If you have any tips for reducing a large and complex multi-file config (where I can't just glob everything into one file, and modifying it in place would cause serious issues because I also need to use neovim to, well, edit and work on other stuff, including the minimal reproduction itself), I'd appreciate it ^.^ - in particular, how does I've found additional evidence that it's related to invalid bufnrs somehow remaining inside EDIT: I've discovered |
Trying to make a |
So, I've had a look for all references to Either that, or a buffer is being detached when it should not be, then being somehow reattached. The detaching process cleans up all callbacks but leaves the list of callbacks in a valid state that could theoretically be re-appended to (just from a cursory glance, idk if this is true for sure :p) The only places where that function is called with
There are also several places where Another note: Reloading a file with |
I managed to figure out a change to my (minimal) config that fixes the issue. So I can hopefully reproduce it now! (the short story is that having Hopefully, I will have a EDIT: still having some trouble, but I will try and make it so the filetype autocmd is triggered during textlock, as my current setup of manually creating a scratchbuffer is not sufficient. I need to sleep though, but I think we might actually be able to reproduce this at somepoint! And even if not, I have managed to fix it for my own config by wrapping Basically, in some situations, if you don't wrap |
guys, thanks so much for looking into this, seems like this has fixed it - and even for say a file type lua which i do not even have a callback such as below. e.g. https://github.com/TimelordUK/configs/blob/main/.config/nvim/lua/plugins/lspconfig.lua local function start_lsp(cfg)
vim.schedule(function()
vim.lsp.start(cfg)
end)
end
vim.api.nvim_create_autocmd('FileType', {
pattern = 'cpp',
callback = function()
start_lsp({
name = 'clangd',
cmd = { 'clangd' },
})
end,
}) |
Here's a minimal repro: https://github.com/icholy/nvim-lsp-bug-4 @zeertzjq can you re-open this? |
The neovim/runtime/lua/vim/lsp.lua Line 580 in 4f24e1b
|
Can you try with Or actually, we shouldn't send changes for the uninitialized clients. But we shouldn't detach either. |
@mfussenegger yeah that works. diff --git a/runtime/lua/vim/lsp.lua b/runtime/lua/vim/lsp.lua
index 1592fd315..60b3f3e50 100644
--- a/runtime/lua/vim/lsp.lua
+++ b/runtime/lua/vim/lsp.lua
@@ -577,7 +577,8 @@ local function buf_attach(bufnr)
api.nvim_buf_attach(bufnr, false, {
on_lines = function(_, _, changedtick, firstline, lastline, new_lastline)
if #lsp.get_clients({ bufnr = bufnr }) == 0 then
- return true -- detach
+ -- detach if there are no clients
+ return #lsp.get_clients({ bufnr = bufnr, _uninitialized = true }) == 0
end
util.buf_versions[bufnr] = changedtick
changetracking.send_changes(bufnr, firstline, lastline, new_lastline) |
@sapient-cogbag I'm pretty sure it's actually fixed this time |
This fixes the problem for me, I just pulled the latest version. Thanks :) |
Problem
Problem
When using a system to automatically attach buffers to a language server that reinitializes itself, or responds to the
initialize
RPC query slowly, or something similar, if one of those buffers becomes invalid then the_on_attach
method of LSP Clients produces an invalid buffer-number error.This problem is compounded if for one reason or another the initialize query is repeatedly responded to or multiple buffers attached to a client become invalid over time (for example, if significant numbers of ephemeral preview buffers are attached to a language server and then removed in short succession), as all of these invalid buffers are provided to
_on_attach
on what I assume are repeatedinitialize
responses from the LSP server, every time, creating a large error log spam which greatly interferes with typing >.<While this is most acute when using a system that happens to attach and invalidate ephemeral buffers, it is (again, as far as I can tell) a race condition that should apply in any case where an LSP server reinitializes itself after any of it's attached buffers are rendered invalid. However, in practice it seems extremely difficult to reproduce....
Originating Code Path
runtime/lua/vim/lsp/client.lua:608
- this is inside a callback passed torpc.request
with methodinitialize
runtime/lua/vim/lsp/client.lua:938
- this is the actual_on_attach
method. The error comes from inside_text_document_did_open_handler
methodruntime/lua/vim/lsp/client.lua:902
runtime/lua/vim/lsp/_changetracking.lua:149
, inside theM.init
module function.Steps to reproduce using "nvim -u minimal_init.lua"
I could not get a minimal reproduction, unfortunately. I spent many many hours trying. However, minimal reproductions of race conditions are rather difficult. In particular, I've failed to get a minimal config to get an invalid bufnr into the
attached_buffers
of any LSP server, even though I know it is possible because my custom LSP setup (which is a bit of an abomination at this point...) has a status command which I modified to report on that (among other things like listing files with each language server client, filtering by language, etc.)My final failed attempt at
minimal_init.lua
Folder Arrangement
Just placing
minimal_init.lua
in a folder with an empty.editorfile
, anddummy-lua.lua
(the.editorfile
is for lsp sharing) will get the configuration i was using while writing this failed minimal config.Actual Setup Behaviour
It's probably worth noting that the actual setup I have is more complex than this in certain ways that may be relevant. Importantly, the main LSP servers that I see in my listing (
:LspStatus
) command with invalid bufnrs ismarksman
(the markdown LSP I use).My suspicion is that somehow the autocmd I have that automatically produces a popup when typing (which is constantly being refreshed) manages to trigger this race condition. But it also seems to occur more after I use preview windows with
fzf-lua
if they are scrolled through very rapidly (I use no plugins for the popup/documentation aspect, so it's not an fzf-lua thing). However, this is notmarksman
stuff, even if I havent seen it in any kind of:LspStatus
listing yet :/Logs
I have looked in logs, but there doesn't seem to be much useful there (though there are a signifcant number of errors from
marksman
and a few from mylua-language-server
in some of the other earlier loglines. Still:(I don't know what exactly is causing the loop callback error and it seems to sometimes not appear... I think it is not related since it seems to only occur after the first epoch reload I do, whereas the actual issue I'm reporting occurs all of the time - and yes, I have an epoch system for reloading my config... it's about 10x as horrifying as it sounds especially because I was rushing at the time :p)
:LspStatus
(my version) - probably not useful, but putting it here anyway in case it is potentially useful in figuring out the underlying causesExpected behavior
Handle invalid buffers gracefully in whatever way is most conceptually in line with how the LSP infrastructure is meant to work - I don't know the best place to put the fix, even if it is extremely simple (just a check with
vim.api.nvim_buf_is_valid
^.^). It may also not be worth doing as it seems to be quite hard to produce without some pretty weird config stuff, although it might just be that I didn't put the language servers under enough pressure in my many attempts atminimal_init.lua
?It might be related to the todo in
runtime/vim/lsp.lua:70
relating toresolve_bufnr
on scratch buffers, but I really just don't knowFrom a practical perspective, fixing this issue could probably occur in two locations - the
rpc.request
call, and deep in the_changetracker
init method - and it would be a very simple check.Neovim version (nvim -v)
NVIM v0.10.0-dev-2995+ga1550dbf0a (but applies in all versions with relevant code)
Language server name/version
lua-language-server (3.8.3-1), marksman (2023_12_09-1), rust-analyzer (1.79.0-nightly), presumably others
Operating system/version
Arch Linux
Log file
No response
The text was updated successfully, but these errors were encountered: