fix(vim.ui): open() may wait indefinitely #28325

Problem:
vim.ui.open "locks up" Nvim if the spawned process does not terminate. #27986

Solution:
- Change `vim.ui.open()`:
    - Do not call `wait()`.
    - Return a `SystemObj`. The caller can decide if it wants to `wait()`.
- Change `gx` to `wait()` only a short time.
    - Allows `gx` to show a message if the command fails, without the
      risk of waiting forever.

7 files changed, 38 insertions, 20 deletions

diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt
index d967e2b313..c760e762ee 100644
--- a/runtime/doc/lua.txt
+++ b/runtime/doc/lua.txt
@@ -1812,6 +1812,7 @@ vim.system({cmd}, {opts}, {on_exit})                            *vim.system()*
 
     Return: ~
         (`vim.SystemObj`) Object with the fields:
+        • cmd (string[]) Command name and args
         • pid (integer) Process ID
         • wait (fun(timeout: integer|nil): SystemCompleted) Wait for the
           process to complete. Upon timeout the process is sent the KILL
@@ -2568,16 +2569,21 @@ vim.ui.open({path})                                            *vim.ui.open()*
     Expands "~/" and environment variables in filesystem paths.
 
     Examples: >lua
+        -- Asynchronous.
         vim.ui.open("https://neovim.io/")
         vim.ui.open("~/path/to/file")
-        vim.ui.open("$VIMRUNTIME")
+        -- Synchronous (wait until the process exits).
+        local cmd, err = vim.ui.open("$VIMRUNTIME")
+        if cmd then
+          cmd:wait()
+        end
 <
 
     Parameters: ~
       • {path}  (`string`) Path or URL to open
 
     Return (multiple): ~
-        (`vim.SystemCompleted?`) Command result, or nil if not found.
+        (`vim.SystemObj?`) Command object, or nil if not found.
         (`string?`) Error message on failure
 
     See also: ~
diff --git a/runtime/lua/vim/_defaults.lua b/runtime/lua/vim/_defaults.lua
index 533ebbc7c3..5ada64a358 100644
--- a/runtime/lua/vim/_defaults.lua
+++ b/runtime/lua/vim/_defaults.lua
@@ -95,10 +95,19 @@ do
     { silent = true, expr = true, desc = ':help v_@-default' }
   )
 
-  --- Map |gx| to call |vim.ui.open| on the identifier under the cursor
+  --- Map |gx| to call |vim.ui.open| on the <cfile> at cursor.
   do
     local function do_open(uri)
-      local _, err = vim.ui.open(uri)
+      local cmd, err = vim.ui.open(uri)
+      local rv = cmd and cmd:wait(1000) or nil
+      if cmd and rv and rv.code ~= 0 then
+        err = ('vim.ui.open: command %s (%d): %s'):format(
+          (rv.code == 124 and 'timeout' or 'failed'),
+          rv.code,
+          vim.inspect(cmd.cmd)
+        )
+      end
+
       if err then
         vim.notify(err, vim.log.levels.ERROR)
       end
diff --git a/runtime/lua/vim/_editor.lua b/runtime/lua/vim/_editor.lua
index 18f6cfa4ba..a40b298a37 100644
--- a/runtime/lua/vim/_editor.lua
+++ b/runtime/lua/vim/_editor.lua
@@ -121,6 +121,7 @@ vim.log = {
 ---   asynchronously. Receives SystemCompleted object, see return of SystemObj:wait().
 ---
 --- @return vim.SystemObj Object with the fields:
+---   - cmd (string[]) Command name and args
 ---   - pid (integer) Process ID
 ---   - wait (fun(timeout: integer|nil): SystemCompleted) Wait for the process to complete. Upon
 ---     timeout the process is sent the KILL signal (9) and the exit code is set to 124. Cannot
diff --git a/runtime/lua/vim/_system.lua b/runtime/lua/vim/_system.lua
index e97a5fc6c3..d603971495 100644
--- a/runtime/lua/vim/_system.lua
+++ b/runtime/lua/vim/_system.lua
@@ -18,6 +18,7 @@ local uv = vim.uv
 --- @field stderr? string
 
 --- @class vim.SystemState
+--- @field cmd string[]
 --- @field handle? uv.uv_process_t
 --- @field timer?  uv.uv_timer_t
 --- @field pid? integer
@@ -56,6 +57,7 @@ local function close_handles(state)
 end
 
 --- @class vim.SystemObj
+--- @field cmd string[]
 --- @field pid integer
 --- @field private _state vim.SystemState
 --- @field wait fun(self: vim.SystemObj, timeout?: integer): vim.SystemCompleted
@@ -68,6 +70,7 @@ local SystemObj = {}
 --- @return vim.SystemObj
 local function new_systemobj(state)
   return setmetatable({
+    cmd = state.cmd,
     pid = state.pid,
     _state = state,
   }, { __index = SystemObj })
diff --git a/runtime/lua/vim/lsp/handlers.lua b/runtime/lua/vim/lsp/handlers.lua
index daf4fec8d2..1c5291e7fd 100644
--- a/runtime/lua/vim/lsp/handlers.lua
+++ b/runtime/lua/vim/lsp/handlers.lua
@@ -615,7 +615,8 @@ M[ms.window_showDocument] = function(_, result, ctx, _)
 
   if result.external then
     -- TODO(lvimuser): ask the user for confirmation
-    local ret, err = vim.ui.open(uri)
+    local cmd, err = vim.ui.open(uri)
+    local ret = cmd and cmd:wait(2000) or nil
 
     if ret == nil or ret.code ~= 0 then
       return {
diff --git a/runtime/lua/vim/ui.lua b/runtime/lua/vim/ui.lua
index b0e7ca1a35..b6695734a3 100644
--- a/runtime/lua/vim/ui.lua
+++ b/runtime/lua/vim/ui.lua
@@ -114,14 +114,19 @@ end
 --- Examples:
 ---
 --- ```lua
+--- -- Asynchronous.
 --- vim.ui.open("https://neovim.io/")
 --- vim.ui.open("~/path/to/file")
---- vim.ui.open("$VIMRUNTIME")
+--- -- Synchronous (wait until the process exits).
+--- local cmd, err = vim.ui.open("$VIMRUNTIME")
+--- if cmd then
+---   cmd:wait()
+--- end
 --- ```
 ---
 ---@param path string Path or URL to open
 ---
----@return vim.SystemCompleted|nil # Command result, or nil if not found.
+---@return vim.SystemObj|nil # Command object, or nil if not found.
 ---@return string|nil # Error message on failure
 ---
 ---@see |vim.system()|
@@ -152,13 +157,7 @@ function M.open(path)
     return nil, 'vim.ui.open: no handler found (tried: explorer.exe, xdg-open)'
   end
 
-  local rv = vim.system(cmd, { text = true, detach = true }):wait()
-  if rv.code ~= 0 then
-    local msg = ('vim.ui.open: command failed (%d): %s'):format(rv.code, vim.inspect(cmd))
-    return rv, msg
-  end
-
-  return rv, nil
+  return vim.system(cmd, { text = true, detach = true }), nil
 end
 
 return M
diff --git a/test/functional/lua/ui_spec.lua b/test/functional/lua/ui_spec.lua
index 8c9148c6f5..71a04a9ae6 100644
--- a/test/functional/lua/ui_spec.lua
+++ b/test/functional/lua/ui_spec.lua
@@ -1,6 +1,6 @@
 local t = require('test.functional.testutil')()
 local eq = t.eq
-local matches = t.matches
+local ok = t.ok
 local exec_lua = t.exec_lua
 local clear = t.clear
 local feed = t.feed
@@ -138,13 +138,12 @@ describe('vim.ui', function()
   describe('open()', function()
     it('validation', function()
       if is_os('win') or not is_ci('github') then
-        exec_lua [[vim.system = function() return { wait=function() return { code=3} end } end]]
+        exec_lua [[vim.system = function() return { wait=function() return { code=3 } end } end]]
       end
       if not is_os('bsd') then
-        matches(
-          'vim.ui.open: command failed %(%d%): { "[^"]+", .*"non%-existent%-file" }',
-          exec_lua [[local _, err = vim.ui.open('non-existent-file') ; return err]]
-        )
+        local rv =
+          exec_lua [[local cmd = vim.ui.open('non-existent-file'); return cmd:wait(100).code]]
+        ok(type(rv) == 'number' and rv ~= 0, 'nonzero exit code', rv)
       end
 
       exec_lua [[