libluv: use luv_set_callback to control callback execution

Disable the use of deferred API functions in a fast lua callback
Correctly display error messages from a fast lua callback

8 files changed, 192 insertions, 14 deletions

diff --git a/runtime/doc/if_lua.txt b/runtime/doc/if_lua.txt
index cbc19a63a2..6dcf3f9fa9 100644
--- a/runtime/doc/if_lua.txt
+++ b/runtime/doc/if_lua.txt
@@ -382,20 +382,26 @@ management.  Try this command to see available functions: >
 See http://docs.libuv.org for complete documentation.
 See https://github.com/luvit/luv/tree/master/examples for examples.
 
-Note: it is not safe to directly invoke the Nvim API from `vim.loop`
-callbacks.  This will crash: >
+						*E5560* *lua-loop-callbacks*
+Note: it is not allowed to directly invoke most of the Nvim API from `vim.loop`
+callbacks.  This will result in an error: >
 
     local timer = vim.loop.new_timer()
     timer:start(1000, 0, function()
-      vim.api.nvim_command('sleep 100m')  -- BROKEN, will crash.
+      vim.api.nvim_command('echomsg "test"')
     end)
 
-Instead wrap the API call with |vim.schedule()|. >
+The `vim.schedule_wrap` helper can be used to defer the callback until it
+is safe to execute API methods. >
 
     local timer = vim.loop.new_timer()
-    timer:start(1000, 0, function()
-      vim.schedule(function() vim.api.nvim_command('sleep 100m') end)
-    end)
+    timer:start(1000, 0, vim.schedule_wrap(function()
+      vim.api.nvim_command('echomsg "test"')
+    end))
+
+A subset of the API is available in direct luv callbacks ("fast" callbacks),
+most notably |nvim_get_mode()| and |nvim_input()|.
+
 
 Example: repeating timer
     1. Save this code to a file.
diff --git a/src/nvim/generators/gen_api_dispatch.lua b/src/nvim/generators/gen_api_dispatch.lua
index 969c19aedb..3ebcd2d1c4 100644
--- a/src/nvim/generators/gen_api_dispatch.lua
+++ b/src/nvim/generators/gen_api_dispatch.lua
@@ -349,6 +349,7 @@ output:write([[
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
 #include "nvim/lua/converter.h"
+#include "nvim/lua/executor.h"
 ]])
 include_headers(output, headers)
 output:write('\n')
@@ -372,6 +373,14 @@ local function process_function(fn)
     binding=lua_c_function_name,
     api=fn.name
   }
+
+  if not fn.fast then
+    write_shifted_output(output, string.format([[
+    if (!nlua_is_deferred_safe(lstate)) {
+      return luaL_error(lstate, e_luv_api_disabled, "%s");
+    }
+    ]], fn.name))
+  end
   local cparams = ''
   local free_code = {}
   for j = #fn.parameters,1,-1 do
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index 0b5fafd7c0..139ffe2144 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -1061,6 +1061,9 @@ EXTERN char_u e_cmdmap_key[] INIT(=N_(
 EXTERN char_u e_api_error[] INIT(=N_(
     "E5555: API call: %s"));
 
+EXTERN char e_luv_api_disabled[] INIT(=N_(
+    "E5560: %s must not be called in a lua loop callback"));
+
 EXTERN char_u e_floatonly[] INIT(=N_(
     "E5601: Cannot close window, only floating window would remain"));
 EXTERN char_u e_floatexchange[] INIT(=N_(
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
index ae872c1540..0127bfae7c 100644
--- a/src/nvim/lua/executor.c
+++ b/src/nvim/lua/executor.c
@@ -33,6 +33,8 @@
 
 #include "luv/luv.h"
 
+static int in_fast_callback = 0;
+
 typedef struct {
   Error err;
   String lua_err_str;
@@ -110,6 +112,50 @@ static int nlua_stricmp(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL
   return 1;
 }
 
+static void nlua_luv_error_event(void **argv)
+{
+  char *error = (char *)argv[0];
+  msg_ext_set_kind("lua_error");
+  emsgf_multiline("Error executing luv callback:\n%s", error);
+  xfree(error);
+}
+
+static int nlua_luv_cfpcall(lua_State *lstate, int nargs, int nresult,
+                            int flags)
+  FUNC_ATTR_NONNULL_ALL
+{
+  int retval;
+
+  // luv callbacks might be executed at any os_breakcheck/line_breakcheck
+  // call, so using the API directly here is not safe.
+  in_fast_callback++;
+
+  int top = lua_gettop(lstate);
+  int status = lua_pcall(lstate, nargs, nresult, 0);
+  if (status) {
+    if (status == LUA_ERRMEM && !(flags & LUVF_CALLBACK_NOEXIT)) {
+      // consider out of memory errors unrecoverable, just like xmalloc()
+      mch_errmsg(e_outofmem);
+      mch_errmsg("\n");
+      preserve_exit();
+    }
+    const char *error = lua_tostring(lstate, -1);
+
+    multiqueue_put(main_loop.events, nlua_luv_error_event,
+                   1, xstrdup(error));
+    lua_pop(lstate, 1);  // error mesage
+    retval = -status;
+  } else {  // LUA_OK
+    if (nresult == LUA_MULTRET) {
+      nresult = lua_gettop(lstate) - top + nargs + 1;
+    }
+    retval = nresult;
+  }
+
+  in_fast_callback--;
+  return retval;
+}
+
 static void nlua_schedule_event(void **argv)
 {
   LuaRef cb = (LuaRef)(ptrdiff_t)argv[0];
@@ -180,6 +226,7 @@ static int nlua_state_init(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL
 
   // vim.loop
   luv_set_loop(lstate, &main_loop.uv);
+  luv_set_callback(lstate, nlua_luv_cfpcall);
   luaopen_luv(lstate);
   lua_setfield(lstate, -2, "loop");
 
@@ -546,6 +593,13 @@ Object executor_exec_lua_cb(LuaRef ref, const char *name, Array args,
   }
 }
 
+/// check if the current execution context is safe for calling deferred API
+/// methods. Luv callbacks are unsafe as they are called inside the uv loop.
+bool nlua_is_deferred_safe(lua_State *lstate)
+{
+  return in_fast_callback == 0;
+}
+
 /// Run lua string
 ///
 /// Used for :lua.
diff --git a/src/nvim/lua/vim.lua b/src/nvim/lua/vim.lua
index 848bccaae6..46c96b455f 100644
--- a/src/nvim/lua/vim.lua
+++ b/src/nvim/lua/vim.lua
@@ -172,11 +172,22 @@ local function __index(t, key)
   end
 end
 
+--- Defers the wrapped callback until when the nvim API is safe to call.
+---
+--- See |vim-loop-callbacks|
+local function schedule_wrap(cb)
+  return (function (...)
+    local args = {...}
+    vim.schedule(function() cb(unpack(args)) end)
+  end)
+end
+
 local module = {
   _update_package_paths = _update_package_paths,
   _os_proc_children = _os_proc_children,
   _os_proc_info = _os_proc_info,
   _system = _system,
+  schedule_wrap = schedule_wrap,
 }
 
 setmetatable(module, {
diff --git a/test/functional/helpers.lua b/test/functional/helpers.lua
index e7a93238e7..95bff21ff5 100644
--- a/test/functional/helpers.lua
+++ b/test/functional/helpers.lua
@@ -686,6 +686,10 @@ local curbufmeths = create_callindex(curbuf)
 local curwinmeths = create_callindex(curwin)
 local curtabmeths = create_callindex(curtab)
 
+local function exec_lua(code, ...)
+  return meths.execute_lua(code, {...})
+end
+
 local function redir_exec(cmd)
   meths.set_var('__redir_exec_cmd', cmd)
   nvim_command([[
@@ -778,6 +782,7 @@ local module = {
   curwinmeths = curwinmeths,
   eval = nvim_eval,
   exc_exec = exc_exec,
+  exec_lua = exec_lua,
   expect = expect,
   expect_any = expect_any,
   expect_msg_seq = expect_msg_seq,
diff --git a/test/functional/lua/loop_spec.lua b/test/functional/lua/loop_spec.lua
index 8cc54e8c13..3b3b81f886 100644
--- a/test/functional/lua/loop_spec.lua
+++ b/test/functional/lua/loop_spec.lua
@@ -1,11 +1,15 @@
 -- Test suite for testing interactions with API bindings
 local helpers = require('test.functional.helpers')(after_each)
+local Screen = require('test.functional.ui.screen')
 local funcs = helpers.funcs
 local meths = helpers.meths
 local clear = helpers.clear
 local sleep = helpers.sleep
+local feed = helpers.feed
 local eq = helpers.eq
+local eval = helpers.eval
 local matches = helpers.matches
+local exec_lua = helpers.exec_lua
 
 before_each(clear)
 
@@ -17,7 +21,7 @@ describe('vim.loop', function()
   end)
 
   it('timer', function()
-    meths.execute_lua('vim.api.nvim_set_var("coroutine_cnt", 0)', {})
+    exec_lua('vim.api.nvim_set_var("coroutine_cnt", 0)', {})
 
     local code=[[
       local loop = vim.loop
@@ -27,14 +31,14 @@ describe('vim.loop', function()
         local this = coroutine.running()
         assert(this)
         local timer = loop.new_timer()
-        timer:start(ms, 0, function ()
+        timer:start(ms, 0, vim.schedule_wrap(function ()
           timer:close()
           touch = touch + 1
           coroutine.resume(this)
           touch = touch + 1
           assert(touch==3)
           vim.api.nvim_set_var("coroutine_cnt_1", touch)
-        end)
+        end))
         coroutine.yield()
         touch = touch + 1
         return touch
@@ -47,9 +51,95 @@ describe('vim.loop', function()
     ]]
 
     eq(0, meths.get_var('coroutine_cnt'))
-    meths.execute_lua(code, {})
-    sleep(20)
+    exec_lua(code)
+    sleep(50)
     eq(2, meths.get_var('coroutine_cnt'))
     eq(3, meths.get_var('coroutine_cnt_1'))
   end)
+
+  it('is API safe', function()
+    local screen = Screen.new(50,10)
+    screen:attach()
+    screen:set_default_attr_ids({
+      [1] = {bold = true, foreground = Screen.colors.Blue1},
+      [2] = {bold = true, reverse = true},
+      [3] = {foreground = Screen.colors.Grey100, background = Screen.colors.Red},
+      [4] = {bold = true, foreground = Screen.colors.SeaGreen4},
+      [5] = {bold = true},
+    })
+
+    -- deferred API functions are disabled, as their safety can't be guaranteed
+    exec_lua([[
+      local timer = vim.loop.new_timer()
+      timer:start(20, 0, function ()
+        timer:close()
+        vim.api.nvim_set_var("valid", true)
+        vim.api.nvim_command("echomsg 'howdy'")
+      end)
+    ]])
+
+    screen:expect([[
+                                                        |
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {2:                                                  }|
+      {3:Error executing luv callback:}                     |
+      {3:[string "<nvim>"]:4: E5560: nvim_set_var must not }|
+      {3:be called in a lua loop callback}                  |
+      {4:Press ENTER or type command to continue}^           |
+    ]])
+    feed('<cr>')
+    eq(false, eval("get(g:, 'valid', v:false)"))
+
+    -- callbacks can be scheduled to be executed in the main event loop
+    -- where the entire API is available
+    exec_lua([[
+      local timer = vim.loop.new_timer()
+      timer:start(20, 0, vim.schedule_wrap(function ()
+        timer:close()
+        vim.api.nvim_set_var("valid", true)
+        vim.api.nvim_command("echomsg 'howdy'")
+      end))
+    ]])
+
+    screen:expect([[
+      ^                                                  |
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      howdy                                             |
+    ]])
+    eq(true, eval("get(g:, 'valid', v:false)"))
+
+    -- fast (not deferred) API functions are allowed to be called directly
+    exec_lua([[
+      local timer = vim.loop.new_timer()
+      timer:start(20, 0, function ()
+        timer:close()
+        -- input is queued for processing after the callback returns
+        vim.api.nvim_input("isneaky")
+        _G.mode = vim.api.nvim_get_mode()
+      end)
+    ]])
+    screen:expect([[
+      sneaky^                                            |
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {5:-- INSERT --}                                      |
+    ]])
+    eq({blocking=false, mode='n'}, exec_lua("return _G.mode"))
+  end)
 end)
diff --git a/third-party/CMakeLists.txt b/third-party/CMakeLists.txt
index d789e48299..5db8e10a33 100644
--- a/third-party/CMakeLists.txt
+++ b/third-party/CMakeLists.txt
@@ -154,9 +154,9 @@ set(LIBTERMKEY_SHA256 cecbf737f35d18f433c8d7864f63c0f878af41f8bd0255a3ebb16010dc
 set(LIBVTERM_URL https://github.com/neovim/libvterm/archive/b45b648cab73f9667bde7c0c6045b285e22b3ecd.tar.gz)
 set(LIBVTERM_SHA256 37cc123deff29327efa654358c2ebaaf8589da03754ca5adb8ec47be386a0433)
 
-set(LUV_VERSION 1.29.1-2)
+set(LUV_VERSION 1.30.0-0)
 set(LUV_URL https://github.com/luvit/luv/archive/${LUV_VERSION}.tar.gz)
-set(LUV_SHA256 e75d8fd2a14433bb798900a71e45318b3c0b8c2ef2c1c43593482ce95b4999e2)
+set(LUV_SHA256 6e468fa17bf222ca8ce0bfffdbdf947fc897da48643a12955db92f80e2c852f5)
 
 set(LUA_COMPAT53_URL https://github.com/keplerproject/lua-compat-5.3/archive/v0.7.tar.gz)
 set(LUA_COMPAT53_SHA256 bec3a23114a3d9b3218038309657f0f506ad10dfbc03bb54e91da7e5ffdba0a2)