loop __gc is not set when luv_set_loop is used.

[lewis6991]

Neovim sets up luv via:

  if (is_standalone) {
    // do nothing, use libluv like in a standalone interpreter
  } else if (is_thread) {
    luv_set_callback(lstate, nlua_luv_thread_cb_cfpcall);
    luv_set_thread(lstate, nlua_luv_thread_cfpcall);
    luv_set_cthread(lstate, nlua_luv_thread_cfcpcall);
  } else {
    luv_set_loop(lstate, &main_loop.uv);
    luv_set_callback(lstate, nlua_fast_cfpcall);
  }
  luaopen_luv(lstate);
  lua_pushvalue(lstate, -1);
  lua_setfield(lstate, -3, "uv");

Note here, Neovim uses lua_set_loop when setting up the main thread.

However, #742 has revealed that if luv_set_loop is used, then the _gc metamethod (which points to loop_gc) is never applied due to a NULL check. See here.

In the past this was fine as Neovim had it's own loop closing logic, however #742 moved the thread cleaning logic into this __gc method. This code is private and Neovim has no way of calling it in its own loop closing code.

[truemedian]

I think the right approach here might be exposing a luv_cleanup function in the API that implementers like neovim can call in conjunction with luv_set_loop.

When cleanup was moved into __gc it did not occur to me that the event loop may not actually be owned by luv, which means both the userdata to be garbage collected may not exist, and luv may not be responsible for stopping the uv loop.

[lewis6991]

I opened #756 as an alternative to your suggestion. The idea is that the worker cleanup seems to be unrelated to the loop, so instead create an empty userdata with a __gc method and attach it to the thread's lua_State. Haven't tested and not sure the change fully makes sense, but thought I'd give it a shot.

[truemedian]

It just occurred to me that having work cleanup attached to __gc at all is bug prone. If for some reason you have two separate lua states that are both using luv, if you close one it will free the work states; despite the other lua state potentially still using it.

Such a case is highly unlikely, and I'm really not sure there's a correct solution to that one for luv in general because we also have to be able to support Lua just requireing luv.

[lewis6991]

I'm not sure if lua_work_init even works with multiple lua_States so this might be a limitation.

[truemedian]

I had a slightly crazy idea that may work. We could move the list of work vms entirely into a a userdata so that this issue just doesn't exist at all. That way even threads get their own work vms that they can clean up separately.