Windows: treesitter language parsers are not released, preventing reinstallation

[FlippingBinary]

Problem

Neovim opens language parsers in a way that blocks other processes from overwriting them, and does not release the associated file handle until Neovim closes. This prevents the reinstallation and upgrade of language parsers on Windows after the parser has been loaded.

Steps to reproduce

These reproduction steps use lazy.nvim and nvim-treesitter, but manually installing a parser where Neovim can pick it up will trigger the error as well.

1. Install a language parser like :TSInstall rust, or manually by installing it in the correct directory for Neovim to find it.
2. Close Neovim if it was open, then open a file that uses an installed parser.
3. Inspect the tree with :InspectTree (this is just one way of being sure the language parser was loaded)
4. Close the tree and the buffer if you want, but keep the same instance of Neovim open.
5. Attempt to reinstall the language parser (or manually replace it).
6. Observe an error indicating access is denied.

I'm not sure why, but reinstalling immediately after the initial installation results in no message at all.

For reference, I reproduced this initially with a simple init.lua:

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not (vim.uv or vim.loop).fs_stat(lazypath) then
	vim.fn.system({
		"git",
		"clone",
		"--filter=blob:none",
		"https://github.com/folke/lazy.nvim.git",
		"--branch=stable", -- latest stable release
		lazypath,
	})
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
	"nvim-treesitter/nvim-treesitter",
	branch = "main",
	build = ":TSUpdate",
	lazy = false,
})

Then I tried to rule out the involvement of nvim-treesitter and even lazy.nvim by cleaning out the configuration and data files, leaving behind only C:\Users\User\AppData\Local\nvim-data\site, which held the parser in parser\rust.so. Then I just used a sanity-check init.lua:

print("Hello, world!")

The problem still occurred.

Windows has a way of locking files, depending on how they are opened, so it seems like that is causing the problem, which is made worse because they don't seem to be closed until Neovim itself is closed.

Expected behavior

I would expect treesitter language parsers to be replaceable, even when in use, so they can always be upgraded from within Neovim.

Neovim version (nvim -v)

0.10.0-dev-2976+g208852126

Vim (not Nvim) behaves the same?

no?

Operating system/version

Windows 10 Pro 22H2

Terminal name/version

Windows Terminal 1.19.10821.0

$TERM environment variable

N/A - it's empty

Installation

Downloaded nvim-win64.zip from the repository's release assets, but it also exists in 0.9.x versions installed with winget, including 0.9.5.

[clason]

I'm not sure there's much we can do here, to be honest. Tree-sitter itself is not eager in releasing parsers, and we cache them for hard won performance reasons.

I general, Neovim doesn't really support hot reloading, and this is a corollary of that.

[justinmk]

Neovim doesn't really support hot reloading, and this is a corollary of that.

In this case, on Windows, (potentially) all Nvim processes would need to be closed in order to update a TS parser. That's a bit different than not supporting hot-reload.

[clason]

That is fair. Implementing proper garbage collection would be a significant overhead, though.

[FlippingBinary]

@justinmk Correct. If any Neovim process has opened it at least once during its lifetime, the language parser cannot be deleted until that process ends or calls uv_dlclose on it. However, it can be moved and renamed as long as it remains on the same filesystem. Microsoft's advice is to move it out of the way, install the new one, then use MoveFileExA with the MOVEFILE_DELAY_UNTIL_REBOOT flag to tell Windows to delete the file automatically during the next reboot.

@clason For Neovim to facilitate proper reinstallation of language parsers on Windows, it would have to be modified to keep track of the file handle it creates with uv_dlopen and also allow other processes to tell it to call uv_dlclose on them, but then there's the issue of when to load the language parser again. Should it trust that a follow-up message will arrive with news that the parser can be loaded? Or watch the filesystem for changes that may never come because of unrelated installation issues? Plus, the plugins would still have to be modified to support the new functionality. It would be much simpler to expect plugins to follow Microsoft's recommended three step process for reinstallation of dynamic-link libraries.

[justinmk]

We can at least ensure that our parser object (er... "LanguageTree") has an API for releasing/destroying parsers. Making it work "properly" might require some work (maybe an upstream TS patch), it sounds like.

[clason]

It would be much simpler to expect plugins to follow Microsoft's recommended three step process for reinstallation of dynamic-link libraries.

I don't quite agree. People do expect that installed plugins/whatever are available immediately, and we shouldn't paper over that: Updating a parser that is in use is just not well-defined. And speaking of nvim-treesitter: it doesn't load parsers itself; that is handled by Neovim. It will be loaded when it is requested and is not yet loaded.

We can at least ensure that our parser object (er... "LanguageTree") has an API for releasing/destroying parsers.

There is a tree-sitter API for that which is exposed (privately); the overhead is for tracking "use" of the parser, which we currently don't do at all. It's in fact not so obvious when a parser is to no longer "used" for a buffer (e.g., if injections are created or destroyed).

[FlippingBinary]

I don't quite agree. People do expect that installed plugins/whatever are available immediately, and we shouldn't paper over that: Updating a parser that is in use is just not well-defined. And speaking of nvim-treesitter: it doesn't load parsers itself; that is handled by Neovim. It will be loaded when it is requested and is not yet loaded.

I agree that people do expect that installed components are available immediately, but I'm not suggesting something that would interfere with that goal. Quite the contrary. The current behavior on Windows when reinstalling a loaded language parser with nvim-treesitter is that the new parser does not take effect at all, even after restarting Neovim. It relies on the user to identify which parser failed to install, close all affected instances of Neovim, and launch a call to :TSInstall <language> in a new instance before the language parser is loaded. This is very far from meeting that expectation of having it available immediately. It's a great goal, but the problem I'm highlighting is easy to fix in a way that brings the behavior on Windows in line with the behavior on Linux with the added ability to let the user know that the change didn't take effect immediately. On Linux, the user would remain unaware because nvim-treesitter just replaces the language parser on disk without having the ability to tell Neovim to reload it. Still, asking the user to restart Neovim to finalize a change is a whole lot better than failing to reinstall a parser.

@justinmk If Neovim adds an API just for releasing or destroying language parsers, it would still have to coordinate with every running Neovim process and there is no guarantee that all of them would be capable of releasing a lock (there are many reasons why Neovim might freeze up, especially on Windows). Otherwise, the old parser would remain locked, and the algorithm I outlined in the nvim-treesitter repository would still need to be followed.

To fix the bug on Windows, something -- either nvim-treesitter or Neovim -- would have to move the old one out of the way, install the new one, then either delete the stale one or flag it for deletion on reboot.

For Neovim to fix it, an API could be added for copying new language parsers to an installation location. Then Neovim could reliably replace the old parser (renaming and flagging the old one for deletion on Windows, if necessary), and immediately load the new one. Adding this functionality in Neovim would be beneficial to all users because it would bring with it the ability to immediately load a new language parser, which is lacking on all platforms.

[clason]

For Neovim to fix it, an API could be added for copying new language parsers to an installation location. Then Neovim could reliably replace the old parser (renaming and flagging the old one for deletion on Windows, if necessary), and immediately load the new one. Adding this functionality in Neovim would be beneficial to all users because it would bring with it the ability to immediately load a new language parser, which is lacking on all platforms.

This is something libuv should handle. If you can find out the right flags for that, I'll happily use them on Windows.

[FlippingBinary]

This is something libuv should handle. If you can find out the right flags for that, I'll happily use them on Windows.

The correct flag when using MoveFileExA from the Win32 API is MOVEFILE_DELAY_UNTIL_REBOOT, using the process shown below that I copied from Microsoft's article on Dynamic-Link Library Updates.

It is not necessary to restart the computer if you perform the following steps:

1. Use the MoveFileEx function to rename the DLL being replaced. Do not specify MOVEFILE_COPY_ALLOWED, and make sure the renamed file is on the same volume that contains the original file. You could also simply rename the file in the same directory by giving it a different extension.
2. Copy the new DLL to the directory that contains the renamed DLL. All applications will now use the new DLL.
3. Use MoveFileEx with MOVEFILE_DELAY_UNTIL_REBOOT to delete the renamed DLL.

MoveFileEx takes three arguments: The path to move from, the path to move to, and the flags. To delete a file, just use NULL for the second argument.

For example, using szDeleteFile to hold the path of the file that needs to be deleted on reboot:

MoveFileEx(szDeleteFile, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);

What it actually does is create a registry entry, so the success or failure of the command indicates its ability to create that registry entry.

[clason]

That's not a libuv API, though -- and we use those exclusively.

[FlippingBinary]

Okay, I've looked into libuv's method of deleting files. It looks like their uv_fs_unlink ultimately results in a call to Win32's NtSetInformationFile with a FILE_DISPOSITION_INFORMATION object containing a delete flag. According to Microsoft's remarks on that structure, the file is deleted when the last file handle to it is closed if it can't be deleted immediately. This seems better than the MoveFileEx method, if it truly works.

So simply deleting the old one with uv_fs_unlink should cause it to be deleted automatically when the last handle is closed. Moving the old one to a new location before deleting it with uv_fs_unlink would allow the new parser to be installed in the meantime.

[clason]

Ok, so we don't need to manually clean up. That makes this much more tenable on the nvim-treesitter side. I'll see what I can do.