move luv_work_cleanup into loop gc

[truemedian]

atexit may be called after luv is unloaded and will segfault.

Associated #712

[squeek502]

Are the submodule changes intentional?

[truemedian]

No they most certainly are not, I thought I fixed that

[zhaozg]

LGTM

[clason]

Next necro-post (sorry!) This seems to have introduced an ASAN failure in Neovim tests:
https://github.com/neovim/neovim/actions/runs/12945549440/job/36109345313?pr=32174

Relevant log output:

ERR 2025-01-24T08:32:30.899 T3392.18644.0 loop_close:170: uv_loop_close() hang?
[--I] signal   0x55b8e47b6148
[-AI] async    0x55b8e47b5f90
[RA-] timer    0x50e000000200

(Not very useful; will need to spend more time tracking this down.)

[luukvbaal]

(Not very useful; will need to spend more time tracking this down.)

The relevant ASAN failures in that log are e.g. here: https://github.com/neovim/neovim/actions/runs/12945549440/job/36109345313?pr=32174#step:10:4618. (Search for sanitizer)

[truemedian]

I'm not really sure I understand the test-flow of neovim, but there being many different ASAN logs from what appears to be the same test file, is neovim closing the primary Lua state and reinitializing from scratch between each test?

If this is actually a bug in Luv and not neovim just not calling lua_close on the primary thread (which I think is the only way for this bug to manifest if it wasn't before), then it's worth making this an issue so it can be properly tracked.

[clason]

Yes, the test setup is a bit involved; basically there are two Neovim instances: The one handling the tests (think of this as a lua interpreter with a custom stdlib) and the one that is executing the actual test case ("system under test") which is being started and observed by the test handler; the system under test is where the logs are coming from, and there's indeed a fresh instance for every test (hence many logs).

This is definitely a regression in v1.50.0; if there's a breaking change where something needs to be changed on Neovim's side, I would appreciate more detailed pointers.

[truemedian]

The only thing this change does is move the work vm cleanup from an atexit hook (which causes problems in environments where atexit is called after unloading dynamic libraries) to the __gc hook for the luv loop (and the only way to release the luv loop is to close the lua state).

Therefore if previously neovim was relying on the atexit hook to clean up the work vms and not calling lua_close, then it needs to call lua_close on the primary state now.

If that doesn't solve the problem then this or something else introduced an actual problem and a real issue should be opened.

[lewis6991]

Neovim is calling lua_close here inside nlua_common_free_all_mem which is:

• (for worker threads) set as the thread callback (via luv_set_thread_cb) here
• (for the primary thread) called in nlua_free_all_mem here. This is called whenever Nvim exits and is required for the sanitizers to work.

So from what I can tell, Nvim is always calling lua_close where it needs to.

[lewis6991]

Debugged this a little further. I believe the issue is due to Nvim calling luv_set_loop() (to set our own loop object) right before luaopen_luv() and loop_gc is only set on loops created by Luv. Unfortunetly there's nothing Nvim can do to workaround this since:

  /* do cleanup in main thread */
  lua_getglobal(L, "_THREAD");
  if (lua_isnil(L, -1))
    luv_work_cleanup();
  lua_pop(L, 1);

isn't accessible.

Either the above snippet needs to be wrapped into a function and exposed as API, or luv_work_cleanup() needs to be exposed as API.

[Bilal2453]

Can you open an issue for discussing this?

[lewis6991]

👍 I've opened #754