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/api/private/dispatch.h b/src/nvim/api/private/dispatch.h index 39aabd708a..bad5a13934 100644 --- a/src/nvim/api/private/dispatch.h +++ b/src/nvim/api/private/dispatch.h @@ -11,8 +11,10 @@ typedef Object (*ApiDispatchWrapper)(uint64_t channel_id, /// functions of this type. typedef struct { ApiDispatchWrapper fn; - bool async; // function is always safe to run immediately instead of being - // put in a request queue for handling when nvim waits for input. + bool fast; // Function is safe to be executed immediately while running the + // uv loop (the loop is run very frequently due to breakcheck). + // If "fast" is false, the function is deferred, i e the call will + // be put in the event queue, for safe handling later. } MsgpackRpcRequestHandler; #ifdef INCLUDE_GENERATED_DECLARATIONS diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 72eb68b650..8e5650633a 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -209,7 +209,7 @@ void nvim_feedkeys(String keys, String mode, Boolean escape_csi) /// @return Number of bytes actually written (can be fewer than /// requested if the buffer becomes full). Integer nvim_input(String keys) - FUNC_API_SINCE(1) FUNC_API_ASYNC + FUNC_API_SINCE(1) FUNC_API_FAST { return (Integer)input_enqueue(keys); } @@ -238,7 +238,7 @@ Integer nvim_input(String keys) /// @param[out] err Error details, if any void nvim_input_mouse(String button, String action, String modifier, Integer grid, Integer row, Integer col, Error *err) - FUNC_API_SINCE(6) FUNC_API_ASYNC + FUNC_API_SINCE(6) FUNC_API_FAST { if (button.data == NULL || action.data == NULL) { goto error; @@ -1256,7 +1256,7 @@ Dictionary nvim_get_color_map(void) /// /// @returns Dictionary { "mode": String, "blocking": Boolean } Dictionary nvim_get_mode(void) - FUNC_API_SINCE(2) FUNC_API_ASYNC + FUNC_API_SINCE(2) FUNC_API_FAST { Dictionary rv = ARRAY_DICT_INIT; char *modestr = get_mode(); @@ -1342,7 +1342,7 @@ Dictionary nvim_get_commands(Dictionary opts, Error *err) /// /// @returns 2-tuple [{channel-id}, {api-metadata}] Array nvim_get_api_info(uint64_t channel_id) - FUNC_API_SINCE(1) FUNC_API_ASYNC FUNC_API_REMOTE_ONLY + FUNC_API_SINCE(1) FUNC_API_FAST FUNC_API_REMOTE_ONLY { Array rv = ARRAY_DICT_INIT; @@ -1652,7 +1652,7 @@ typedef kvec_withinit_t(ExprASTConvStackItem, 16) ExprASTConvStack; /// @param[out] err Error details, if any Dictionary nvim_parse_expression(String expr, String flags, Boolean highlight, Error *err) - FUNC_API_SINCE(4) FUNC_API_ASYNC + FUNC_API_SINCE(4) FUNC_API_FAST { int pflags = 0; for (size_t i = 0 ; i < flags.size ; i++) { diff --git a/src/nvim/func_attr.h b/src/nvim/func_attr.h index d3b600a40c..14b8158c7f 100644 --- a/src/nvim/func_attr.h +++ b/src/nvim/func_attr.h @@ -205,8 +205,8 @@ #endif #ifdef DEFINE_FUNC_ATTRIBUTES -/// Non-deferred API function. -# define FUNC_API_ASYNC +/// Fast (non-deferred) API function. +# define FUNC_API_FAST /// Internal C function not exposed in the RPC API. # define FUNC_API_NOEXPORT /// API function not exposed in VimL/eval. diff --git a/src/nvim/generators/c_grammar.lua b/src/nvim/generators/c_grammar.lua index 40bdc3e30c..8d50290ccc 100644 --- a/src/nvim/generators/c_grammar.lua +++ b/src/nvim/generators/c_grammar.lua @@ -35,11 +35,11 @@ local c_params = Ct(c_void + c_param_list) local c_proto = Ct( Cg(c_type, 'return_type') * Cg(c_id, 'name') * fill * P('(') * fill * Cg(c_params, 'parameters') * fill * P(')') * - Cg(Cc(false), 'async') * + Cg(Cc(false), 'fast') * (fill * Cg((P('FUNC_API_SINCE(') * C(num ^ 1)) * P(')'), 'since') ^ -1) * (fill * Cg((P('FUNC_API_DEPRECATED_SINCE(') * C(num ^ 1)) * P(')'), 'deprecated_since') ^ -1) * - (fill * Cg((P('FUNC_API_ASYNC') * Cc(true)), 'async') ^ -1) * + (fill * Cg((P('FUNC_API_FAST') * Cc(true)), 'fast') ^ -1) * (fill * Cg((P('FUNC_API_NOEXPORT') * Cc(true)), 'noexport') ^ -1) * (fill * Cg((P('FUNC_API_REMOTE_ONLY') * Cc(true)), 'remote_only') ^ -1) * (fill * Cg((P('FUNC_API_REMOTE_IMPL') * Cc(true)), 'remote_impl') ^ -1) * diff --git a/src/nvim/generators/gen_api_dispatch.lua b/src/nvim/generators/gen_api_dispatch.lua index f52d05a4a5..3ebcd2d1c4 100644 --- a/src/nvim/generators/gen_api_dispatch.lua +++ b/src/nvim/generators/gen_api_dispatch.lua @@ -309,7 +309,7 @@ for i = 1, #functions do '(String) {.data = "'..fn.name..'", '.. '.size = sizeof("'..fn.name..'") - 1}, '.. '(MsgpackRpcRequestHandler) {.fn = handle_'.. (fn.impl_name or fn.name).. - ', .async = '..tostring(fn.async)..'});\n') + ', .fast = '..tostring(fn.fast)..'});\n') end @@ -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..4fd4e4c4fa 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,8 +226,18 @@ 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"); + lua_pushvalue(lstate, -1); + lua_setfield(lstate, -3, "loop"); + + // package.loaded.luv = vim.loop + // otherwise luv will be reinitialized when require'luv' + lua_getglobal(lstate, "package"); + lua_getfield(lstate, -1, "loaded"); + lua_pushvalue(lstate, -3); + lua_setfield(lstate, -2, "luv"); + lua_pop(lstate, 3); lua_setglobal(lstate, "vim"); return 0; @@ -546,6 +602,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/src/nvim/map.c b/src/nvim/map.c index 90da27cf9f..cdade5ee71 100644 --- a/src/nvim/map.c +++ b/src/nvim/map.c @@ -179,7 +179,7 @@ MAP_IMPL(cstr_t, ptr_t, DEFAULT_INITIALIZER) MAP_IMPL(ptr_t, ptr_t, DEFAULT_INITIALIZER) MAP_IMPL(uint64_t, ptr_t, DEFAULT_INITIALIZER) MAP_IMPL(handle_T, ptr_t, DEFAULT_INITIALIZER) -#define MSGPACK_HANDLER_INITIALIZER { .fn = NULL, .async = false } +#define MSGPACK_HANDLER_INITIALIZER { .fn = NULL, .fast = false } MAP_IMPL(String, MsgpackRpcRequestHandler, MSGPACK_HANDLER_INITIALIZER) #define KVEC_INITIALIZER { .size = 0, .capacity = 0, .items = NULL } MAP_IMPL(HlEntry, int, DEFAULT_INITIALIZER) diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c index c1ad3bd829..81c9f1e3f4 100644 --- a/src/nvim/msgpack_rpc/channel.c +++ b/src/nvim/msgpack_rpc/channel.c @@ -344,7 +344,7 @@ static void handle_request(Channel *channel, msgpack_object *request) evdata->args = args; evdata->request_id = request_id; channel_incref(channel); - if (handler.async) { + if (handler.fast) { bool is_get_mode = handler.fn == handle_nvim_get_mode; if (is_get_mode && !input_blocking()) { 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..cd3b935e1a 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,99 @@ 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 ""]:4: E5560: nvim_set_var must not }| + {3:be called in a lua loop callback} | + {4:Press ENTER or type command to continue}^ | + ]]) + feed('') + 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) + + it("is equal to require('luv')", function() + eq(true, exec_lua("return vim.loop == require('luv')")) + 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)