search

neovim / neovim

Vim-fork focused on extensibility and usability
https://neovim.io
Other
83.17k stars 5.69k forks source link

Crash: vim.uv.walk #25043

Open bukzor opened 1 year ago

bukzor commented 1 year ago

Problem

Using vim.loop.walk in any way results in nvim crashing with a segmentation fault (same for vim.uv.walk).

Steps to reproduce

$ env -i nvim --clean -es +'lua vim.loop.walk(function() print("ohai") end)'
vim: src/loop.c:85: luv_walk_cb: Assertion `data && data->ref < 0x1000000' failed.
Aborted (core dumped)

I got that particular message from v0.4.4. Newer versions crash silently (seems worse, to me).

Expected behavior

Print ohai a bunch of times.

Neovim version (nvim -v)

NVIM v0.10.0-dev-1046+g3afbf4745, NVIM v0.9.1, NVIM v0.4.4

Vim (not Nvim) behaves the same?

n/a

Operating system/version

$ uname -a Linux penguin 5.15.117-19679-g172023e664f7 #1 SMP PREEMPT Fri Aug 18 18:07:47 PDT 2023 x86_64 GNU/Linux

Terminal name/version

hterm

$TERM environment variable

n/a

Installation

source, brew, debian (respectively)

bukzor commented 1 year ago

All versions of neovim (that I have) have a vim.loop.walk but some newer versions also have a vim.uv.walk. The behavior is identical though regardless of which alias is used.

bfredl commented 1 year ago

Libluvs wrapper around uv_walk assumes that libluv (the lua bindings) make exclusive use of the uv event loop. This leads to a crash in nvim as libluv then cannot understand handles that is owned by nvim core, not libluv.

This could be fixed in libluv at the price of making the implementation more complicated. Alternatively, we could change vim.uv.walk to just immediately throw an error.

So I am curious what the usecase of using uv.walk() is here. There might be another way to solve the underlying problem.

bukzor commented 1 year ago

My intent was to iterate through the existing timers and methodically delete them, so as to find which of my plugins is periodically running chdir($HOME). I already did this exercise with the autocmd/augroups but the behavior persisted, and eventually I came to realize the behavior comes from a uv timer. This API looked ideal for that purpose.

While tinkering with this idea I had imagined / hoped for a :Telescope event-loop extension which would let us browse and control timers the way it already does autocmds -- from an end-user perspective they're nearly identical things. vim.uv.walk would be critical for enabling such a thing.

bukzor commented 1 year ago

Libluvs wrapper around uv_walk assumes that libluv (the lua bindings) make exclusive use of the uv event loop.

Would you kindly point out the relevant code? The function I saw takes the event loop as the first argument, so I had expected a method (vim.uv:walk(function...end)).

bfredl commented 1 year ago

Would you kindly point out the relevant code? The function I saw takes the event loop as the first argument, so I had expected a method (vim.uv:walk(function...end)).

It's luv_walk in src/loop.c. libluv keeps track of one uv event loop per lua thread. Indepdendent luv event loops are possible, but they must then run in a separate thread ( vim.uv.new_thread(...)).

While tinkering with this idea I had imagined / hoped for a :Telescope event-loop extension which would let us browse and control timers the way it already does autocmds -- from an end-user perspective they're nearly identical things. vim.uv.walk would be critical for enabling such a thing.

Debugging the event loop is a relevant usecase. But it is going to require some cooperation from nvim core. Such an interface would ideally show and identify uv handles regardless if they come from libluv or nvim core (often wrappers for jobs and timers for vimscript and also RPC channels).

bukzor commented 1 year ago

Is the nvim-core loop exposed to lua at all, currently?

The convention of one-loop-per-lua seems baked into luv pretty deep... are you picturing a major revision to luv or would you not use it for this?

bfredl commented 1 year ago

The convention of one-loop-per-lua seems baked into luv pretty deep... are you picturing a major revision to luv or would you not use it for this?

No not at all. I'm just saying implementing the debugging functionality you want is going to be a bit more involved than just an isolated change to just vim.uv.walk . It is definitely something worth doing and a welcome contribution, I just want to be clear upfront with the scope of the effort needed.

bukzor commented 1 year ago

On Fri, Sep 8, 2023, 3:52 PM bfredl @.***> wrote:

The convention of one-loop-per-lua seems baked into luv pretty deep... are you picturing a major revision to luv or would you not use it for this?

No not at all. I'm just saying implementing the debugging functionality you want is going to be a bit more involved than just an isolated change to just vim.uv.walk . It is definitely something worth doing and a welcome contribution, I just want to be clear upfront with the scope of the effort needed.

— Reply to this email directly, view it on GitHub https://github.com/neovim/neovim/issues/25043#issuecomment-1712209280, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE4KSEQVMVPRYGCPIO5O3LXZOAPTANCNFSM6AAAAAA4POFTEI . You are receiving this because you authored the thread.

Thanks. My question was about the nature of such a hypothetical contribution. Could you outline what you'd expect, in broad strokes?

To my understanding, this would need either:

A. a major revision to luv to allow it to represent/interact with more than one loop per lua vm B. this bit of neovim API wouldn't use luv, and reach more directly to libuv C. a third option that you can think of but I can't

My money is on option C :)

Message ID: @.***>