fix(messages)!: vim.ui_attach message callbacks are unsafe

Problem:  Lua callbacks for "msg_show" events with vim.ui_attach() are
          executed when it is not safe.
Solution: Disallow non-fast API calls for "msg_show" event callbacks.
          Automatically detach callback after excessive errors.
          Make sure fast APIs do not modify Nvim state.

9 files changed, 69 insertions, 43 deletions

diff --git a/src/nvim/api/ui_events.in.h b/src/nvim/api/ui_events.in.h
index 865e84ab91..0ed208fc1a 100644
--- a/src/nvim/api/ui_events.in.h
+++ b/src/nvim/api/ui_events.in.h
@@ -159,7 +159,7 @@ void wildmenu_hide(void)
   FUNC_API_SINCE(3) FUNC_API_REMOTE_ONLY;
 
 void msg_show(String kind, Array content, Boolean replace_last)
-  FUNC_API_SINCE(6) FUNC_API_REMOTE_ONLY;
+  FUNC_API_SINCE(6) FUNC_API_FAST FUNC_API_REMOTE_ONLY;
 void msg_clear(void)
   FUNC_API_SINCE(6) FUNC_API_REMOTE_ONLY;
 void msg_showcmd(Array content)
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 998f911392..e6ec88c5e8 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -594,10 +594,12 @@ ArrayOf(String) nvim_get_runtime_file(String name, Boolean all, Arena *arena, Er
   kvi_init(cookie.rv);
 
   int flags = DIP_DIRFILE | (all ? DIP_ALL : 0);
+  TryState tstate;
+
+  try_enter(&tstate);
+  do_in_runtimepath((name.size ? name.data : ""), flags, find_runtime_cb, &cookie);
+  vim_ignored = try_leave(&tstate, err);
 
-  TRY_WRAP(err, {
-    do_in_runtimepath((name.size ? name.data : ""), flags, find_runtime_cb, &cookie);
-  });
   return arena_take_arraybuilder(arena, &cookie.rv);
 }
 
diff --git a/src/nvim/errors.h b/src/nvim/errors.h
index 7897a71489..df94945a3d 100644
--- a/src/nvim/errors.h
+++ b/src/nvim/errors.h
@@ -151,7 +151,7 @@ EXTERN const char e_auabort[] INIT(= N_("E855: Autocommands caused command to ab
 
 EXTERN const char e_api_error[] INIT(= N_("E5555: API call: %s"));
 
-EXTERN const char e_luv_api_disabled[] INIT(= N_("E5560: %s must not be called in a lua loop callback"));
+EXTERN const char e_fast_api_disabled[] INIT(= N_("E5560: %s must not be called in a fast event context"));
 
 EXTERN const char e_floatonly[] INIT(= N_("E5601: Cannot close window, only floating window would remain"));
 EXTERN const char e_floatexchange[] INIT(= N_("E5602: Cannot exchange or rotate float"));
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 1fb7666167..9acbc05fdf 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -8616,7 +8616,7 @@ bool eval_has_provider(const char *feat, bool throw_if_fast)
   }
 
   if (throw_if_fast && !nlua_is_deferred_safe()) {
-    semsg(e_luv_api_disabled, "Vimscript function");
+    semsg(e_fast_api_disabled, "Vimscript function");
     return false;
   }
 
diff --git a/src/nvim/generators/gen_api_dispatch.lua b/src/nvim/generators/gen_api_dispatch.lua
index 9e0aa407a1..a78f746fee 100644
--- a/src/nvim/generators/gen_api_dispatch.lua
+++ b/src/nvim/generators/gen_api_dispatch.lua
@@ -750,7 +750,7 @@ local function process_function(fn)
     write_shifted_output(
       [[
     if (!nlua_is_deferred_safe()) {
-      return luaL_error(lstate, e_luv_api_disabled, "%s");
+      return luaL_error(lstate, e_fast_api_disabled, "%s");
     }
     ]],
       fn.name
diff --git a/src/nvim/generators/gen_api_ui_events.lua b/src/nvim/generators/gen_api_ui_events.lua
index 3e8ae19c9a..30a83330eb 100644
--- a/src/nvim/generators/gen_api_ui_events.lua
+++ b/src/nvim/generators/gen_api_ui_events.lua
@@ -136,8 +136,8 @@ for i = 1, #events do
       call_output:write('  }\n')
       call_output:write('  entered = true;\n')
       write_arglist(call_output, ev)
-      call_output:write('  ui_call_event("' .. ev.name .. '", ' .. args .. ');\n')
-      call_output:write('  entered = false;\n')
+      call_output:write(('  ui_call_event("%s", %s, %s)'):format(ev.name, tostring(ev.fast), args))
+      call_output:write(';\n  entered = false;\n')
     elseif ev.compositor_impl then
       call_output:write('  ui_comp_' .. ev.name)
       write_signature(call_output, ev, '', true)
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
index e4da274204..15f70fb725 100644
--- a/src/nvim/lua/executor.c
+++ b/src/nvim/lua/executor.c
@@ -187,7 +187,7 @@ static void nlua_luv_error_event(void **argv)
   msg_ext_set_kind("lua_error");
   switch (type) {
   case kCallback:
-    semsg_multiline("Error executing luv callback:\n%s", error);
+    semsg_multiline("Error executing callback:\n%s", error);
     break;
   case kThread:
     semsg_multiline("Error in luv thread:\n%s", error);
@@ -201,13 +201,13 @@ static void nlua_luv_error_event(void **argv)
   xfree(error);
 }
 
-static int nlua_luv_cfpcall(lua_State *lstate, int nargs, int nresult, int flags)
+/// Execute callback in "fast" context. Used for luv and some vim.ui_event
+/// callbacks where using the API directly is not safe.
+static int nlua_fast_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);
@@ -366,11 +366,13 @@ static int nlua_init_argv(lua_State *const L, char **argv, int argc, int lua_arg
 static void nlua_schedule_event(void **argv)
 {
   LuaRef cb = (LuaRef)(ptrdiff_t)argv[0];
+  uint32_t ns_id = (uint32_t)(ptrdiff_t)argv[1];
   lua_State *const lstate = global_lstate;
   nlua_pushref(lstate, cb);
   nlua_unref_global(lstate, cb);
   if (nlua_pcall(lstate, 0, 0)) {
     nlua_error(lstate, _("Error executing vim.schedule lua callback: %.*s"));
+    ui_remove_cb(ns_id, true);
   }
 }
 
@@ -392,8 +394,9 @@ static int nlua_schedule(lua_State *const lstate)
   }
 
   LuaRef cb = nlua_ref_global(lstate, 1);
-
-  multiqueue_put(main_loop.events, nlua_schedule_event, (void *)(ptrdiff_t)cb);
+  // Pass along UI event handler to disable on error.
+  multiqueue_put(main_loop.events, nlua_schedule_event, (void *)(ptrdiff_t)cb,
+                 (void *)(ptrdiff_t)ui_event_ns_id);
   return 0;
 }
 
@@ -425,7 +428,7 @@ static int nlua_wait(lua_State *lstate)
   FUNC_ATTR_NONNULL_ALL
 {
   if (in_fast_callback) {
-    return luaL_error(lstate, e_luv_api_disabled, "vim.wait");
+    return luaL_error(lstate, e_fast_api_disabled, "vim.wait");
   }
 
   intptr_t timeout = luaL_checkinteger(lstate, 1);
@@ -598,7 +601,7 @@ static void nlua_common_vim_init(lua_State *lstate, bool is_thread, bool is_stan
     luv_set_cthread(lstate, nlua_luv_thread_cfcpcall);
   } else {
     luv_set_loop(lstate, &main_loop.uv);
-    luv_set_callback(lstate, nlua_luv_cfpcall);
+    luv_set_callback(lstate, nlua_fast_cfpcall);
   }
   luaopen_luv(lstate);
   lua_pushvalue(lstate, -1);
@@ -724,7 +727,7 @@ static int nlua_ui_detach(lua_State *lstate)
     return luaL_error(lstate, "invalid ns_id");
   }
 
-  ui_remove_cb(ns_id);
+  ui_remove_cb(ns_id, false);
   return 0;
 }
 
@@ -1174,7 +1177,7 @@ int nlua_call(lua_State *lstate)
   size_t name_len;
   const char *name = luaL_checklstring(lstate, 1, &name_len);
   if (!nlua_is_deferred_safe() && !viml_func_is_fast(name)) {
-    return luaL_error(lstate, e_luv_api_disabled, "Vimscript function");
+    return luaL_error(lstate, e_fast_api_disabled, "Vimscript function");
   }
 
   int nargs = lua_gettop(lstate) - 1;
@@ -1231,7 +1234,7 @@ free_vim_args:
 static int nlua_rpcrequest(lua_State *lstate)
 {
   if (!nlua_is_deferred_safe()) {
-    return luaL_error(lstate, e_luv_api_disabled, "rpcrequest");
+    return luaL_error(lstate, e_fast_api_disabled, "rpcrequest");
   }
   return nlua_rpc(lstate, true);
 }
@@ -1594,6 +1597,12 @@ bool nlua_ref_is_function(LuaRef ref)
 Object nlua_call_ref(LuaRef ref, const char *name, Array args, LuaRetMode mode, Arena *arena,
                      Error *err)
 {
+  return nlua_call_ref_ctx(false, ref, name, args, mode, arena, err);
+}
+
+Object nlua_call_ref_ctx(bool fast, LuaRef ref, const char *name, Array args, LuaRetMode mode,
+                         Arena *arena, Error *err)
+{
   lua_State *const lstate = global_lstate;
   nlua_pushref(lstate, ref);
   int nargs = (int)args.size;
@@ -1605,7 +1614,13 @@ Object nlua_call_ref(LuaRef ref, const char *name, Array args, LuaRetMode mode,
     nlua_push_Object(lstate, &args.items[i], 0);
   }
 
-  if (nlua_pcall(lstate, nargs, 1)) {
+  if (fast) {
+    if (nlua_fast_cfpcall(lstate, nargs, 1, -1) < 0) {
+      // error is already scheduled, set anyways to convey failure.
+      api_set_error(err, kErrorTypeException, "fast context failure");
+    }
+    return NIL;
+  } else if (nlua_pcall(lstate, nargs, 1)) {
     // if err is passed, the caller will deal with the error.
     if (err) {
       size_t len;
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index 560f76d0bd..d50747e63f 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -44,6 +44,7 @@
 
 typedef struct {
   LuaRef cb;
+  uint8_t errors;
   bool ext_widgets[kUIGlobalCount];
 } UIEventCallback;
 
@@ -212,21 +213,20 @@ void ui_refresh(void)
   cursor_row = cursor_col = 0;
   pending_cursor_update = true;
 
+  bool had_message = ui_ext[kUIMessages];
   for (UIExtension i = 0; (int)i < kUIExtCount; i++) {
+    ui_ext[i] = ext_widgets[i] | ui_cb_ext[i];
     if (i < kUIGlobalCount) {
-      ext_widgets[i] |= ui_cb_ext[i];
+      ui_call_option_set(cstr_as_string(ui_ext_names[i]), BOOLEAN_OBJ(ui_ext[i]));
     }
-    // Set 'cmdheight' to zero for all tabpages when ext_messages becomes active.
-    if (i == kUIMessages && !ui_ext[i] && ext_widgets[i]) {
-      set_option_value(kOptCmdheight, NUMBER_OPTVAL(0), 0);
-      command_height();
-      FOR_ALL_TABS(tp) {
-        tp->tp_ch_used = 0;
-      }
-    }
-    ui_ext[i] = ext_widgets[i];
-    if (i < kUIGlobalCount) {
-      ui_call_option_set(cstr_as_string(ui_ext_names[i]), BOOLEAN_OBJ(ext_widgets[i]));
+  }
+
+  // Reset 'cmdheight' for all tabpages when ext_messages toggles.
+  if (had_message != ui_ext[kUIMessages]) {
+    set_option_value(kOptCmdheight, NUMBER_OPTVAL(had_message), 0);
+    command_height();
+    FOR_ALL_TABS(tp) {
+      tp->tp_ch_used = had_message;
     }
   }
 
@@ -713,13 +713,15 @@ void ui_grid_resize(handle_T grid_handle, int width, int height, Error *err)
   }
 }
 
-void ui_call_event(char *name, Array args)
+void ui_call_event(char *name, bool fast, Array args)
 {
-  UIEventCallback *event_cb;
   bool handled = false;
-  map_foreach_value(&ui_event_cbs, event_cb, {
+  UIEventCallback *event_cb;
+  map_foreach(&ui_event_cbs, ui_event_ns_id, event_cb, {
     Error err = ERROR_INIT;
-    Object res = nlua_call_ref(event_cb->cb, name, args, kRetNilBool, NULL, &err);
+    uint32_t ns_id = ui_event_ns_id;
+    Object res = nlua_call_ref_ctx(fast, event_cb->cb, name, args, kRetNilBool, NULL, &err);
+    ui_event_ns_id = 0;
     // TODO(bfredl/luukvbaal): should this be documented or reconsidered?
     // Why does truthy return from Lua callback mean remote UI should not receive
     // the event.
@@ -728,6 +730,7 @@ void ui_call_event(char *name, Array args)
     }
     if (ERROR_SET(&err)) {
       ELOG("Error executing UI event callback: %s", err.msg);
+      ui_remove_cb(ns_id, true);
     }
     api_clear_error(&err);
   })
@@ -780,12 +783,16 @@ void ui_add_cb(uint32_t ns_id, LuaRef cb, bool *ext_widgets)
   ui_refresh();
 }
 
-void ui_remove_cb(uint32_t ns_id)
+void ui_remove_cb(uint32_t ns_id, bool checkerr)
 {
-  if (map_has(uint32_t, &ui_event_cbs, ns_id)) {
-    UIEventCallback *item = pmap_del(uint32_t)(&ui_event_cbs, ns_id, NULL);
+  UIEventCallback *item = pmap_get(uint32_t)(&ui_event_cbs, ns_id);
+  if (item && (!checkerr || ++item->errors > 10)) {
+    pmap_del(uint32_t)(&ui_event_cbs, ns_id, NULL);
     free_ui_event_callback(item);
+    ui_cb_update_ext();
+    ui_refresh();
+    if (checkerr) {
+      msg_schedule_semsg("Excessive errors in vim.ui_attach() callback from ns: %d.", ns_id);
+    }
   }
-  ui_cb_update_ext();
-  ui_refresh();
 }
diff --git a/src/nvim/ui.h b/src/nvim/ui.h
index 8718c7b506..4aeb8ffda9 100644
--- a/src/nvim/ui.h
+++ b/src/nvim/ui.h
@@ -18,4 +18,6 @@ EXTERN Array noargs INIT(= ARRAY_DICT_INIT);
 #endif
 // uncrustify:on
 
-EXTERN MultiQueue *resize_events;
+// vim.ui_attach() namespace of currently executed callback.
+EXTERN uint32_t ui_event_ns_id INIT( = 0);
+EXTERN MultiQueue *resize_events INIT( = NULL);