fix(vim.system): wait() timeout, and signal being sent

Problem:

- `vim.system(...):wait(0)` was simply returning `nil` rather than a
  valid `vim.SystemCompleted` object.

- Upon timeout after `:wait(timeout)`, SIGKILL (9) would be sent instead
  of SIGTERM (15) as in the case of `vim.system()` with `timeout` being
  set in the `opts` table. It can be confusing to have different
  behaviors (signals) here.

Solution:

- Document that `timeout` needs to be >= 1.

  Even if an invalid timeout is given, wait for at least 1 milliseconds
  so that the asynchronous `on_exit()` callback can be scheduled and
  executed by the uv loop.

- Always send SIGTERM when the process times out, for both use cases
  involving `timeout`.

runtime/doc/lua.txt

@@ -1780,15 +1780,16 @@ vim.system({cmd}, {opts}, {on_exit})                            *vim.system()*
                      will be written to stdin and closed. Defaults to `false`.
                    • stdout: (boolean|function) Handle output from stdout.
                      When passed as a function must have the signature
-                     `fun(err: string, data: string)`. Defaults to `true`
+                     `fun(err: string, data: string)`. Defaults to `true`.
                    • stderr: (boolean|function) Handle output from stderr.
                      When passed as a function must have the signature
                      `fun(err: string, data: string)`. Defaults to `true`.
                    • text: (boolean) Handle stdout and stderr as text.
                      Replaces `\r\n` with `\n`.
-                   • timeout: (integer) Run the command with a time limit.
-                     Upon timeout the process is sent the TERM signal (15) and
-                     the exit code is set to 124.
+                   • timeout: (integer) Run the command with a time limit
+                     (given in milliseconds). Upon timeout the process is sent
+                     the TERM signal (15) and the exit code is set to 124.
+                     `timeout` must be >= 1.
                    • detach: (boolean) If true, spawn the child process in a
                      detached state - this will make it a process group
                      leader, and will effectively enable the child to keep
@@ -1805,11 +1806,12 @@ vim.system({cmd}, {opts}, {on_exit})                            *vim.system()*
         (`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 be called in
-          |api-fast|.
-          • SystemCompleted is an object with the fields:
+        • wait (fun(timeout: integer|nil): vim.SystemCompleted) Wait for the
+          process to complete, for {timeout} milliseconds (if {timeout} is
+          given) or the timeout specified in {opts}. Upon timeout the process
+          is sent the TERM signal (15) and the exit code is set to 124.
+          {timeout} must be >= 1. Cannot be called in |api-fast|.
+          • vim.SystemCompleted is an object with the fields:
             • code: (integer)
             • signal: (integer)
             • stdout: (string), nil if stdout argument is passed

runtime/lua/vim/_editor.lua

@@ -105,13 +105,14 @@ vim.log = {
 ---     and closed. Defaults to `false`.
 ---   - stdout: (boolean|function)
 ---     Handle output from stdout. When passed as a function must have the signature `fun(err: string, data: string)`.
----     Defaults to `true`
+---     Defaults to `true`.
 ---   - stderr: (boolean|function)
 ---     Handle output from stderr. When passed as a function must have the signature `fun(err: string, data: string)`.
 ---     Defaults to `true`.
 ---   - text: (boolean) Handle stdout and stderr as text. Replaces `\r\n` with `\n`.
----   - timeout: (integer) Run the command with a time limit. Upon timeout the process is sent the
----     TERM signal (15) and the exit code is set to 124.
+---   - timeout: (integer) Run the command with a time limit (given in milliseconds).
+---     Upon timeout the process is sent the TERM signal (15) and the exit code is set to 124.
+---     `timeout` must be >= 1.
 ---   - detach: (boolean) If true, spawn the child process in a detached state - this will make it
 ---     a process group leader, and will effectively enable the child to keep running after the
 ---     parent exits. Note that the child process will still keep the parent's event loop alive
@@ -123,10 +124,11 @@ vim.log = {
 --- @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
----     be called in |api-fast|.
----     - SystemCompleted is an object with the fields:
+---   - wait (fun(timeout: integer|nil): vim.SystemCompleted) Wait for the process to complete,
+---     for {timeout} milliseconds (if {timeout} is given) or the timeout specified in {opts}.
+---     Upon timeout the process is sent the TERM signal (15) and the exit code is set to 124.
+---     {timeout} must be >= 1. Cannot be called in |api-fast|.
+---     - vim.SystemCompleted is an object with the fields:
 ---       - code: (integer)
 ---       - signal: (integer)
 ---       - stdout: (string), nil if stdout argument is passed

runtime/lua/vim/_system.lua

@@ -8,7 +8,7 @@ local uv = vim.uv
 --- @field env? table<string,string|number>
 --- @field clear_env? boolean
 --- @field text? boolean
---- @field timeout? integer Timeout in ms
+--- @field timeout? integer Timeout in ms (>= 1)
 --- @field detach? boolean
 
 --- @class vim.SystemCompleted
@@ -90,21 +90,27 @@ end
 
 local MAX_TIMEOUT = 2 ^ 31
 
---- @param timeout? integer
+--- @param timeout? integer timeout (in milliseconds), must be >= 1.
 --- @return vim.SystemCompleted
 function SystemObj:wait(timeout)
   local state = self._state
 
-  local done = vim.wait(timeout or state.timeout or MAX_TIMEOUT, function()
+  -- If timeout < 1, vim.wait() will return immediately and uv timer won't schedule on_exit(),
+  -- in which state.result will be set. We don't exactly specify the behavior,
+  -- but just terminate the process immediately for now
+  timeout = math.max(1, timeout or state.timeout or MAX_TIMEOUT)
+
+  local done = vim.wait(timeout, function()
     return state.result ~= nil
   end, nil, true)
 
   if not done then
-    -- Send sigkill since this cannot be caught
-    self:_timeout(SIG.KILL)
-    vim.wait(timeout or state.timeout or MAX_TIMEOUT, function()
+    self:_timeout(SIG.TERM)
+    -- Wait a bit more until state.result is set after SIGTERM
+    vim.wait(timeout, function()
       return state.result ~= nil
     end, nil, true)
+    -- TODO: if the process still does not terminate after SIGTERM, send SIGKILL
   end
 
   return state.result

test/functional/lua/system_spec.lua

@@ -75,7 +75,7 @@ describe('vim.system', function()
       it('supports timeout', function()
         eq({
           code = 124,
-          signal = 15,
+          signal = 15, -- SIGTERM
           stdout = '',
           stderr = '',
         }, system({ 'sleep', '10' }, { timeout = 1000 }))
@@ -83,6 +83,18 @@ describe('vim.system', function()
     end)
   end
 
+  it('(sync) supports timeout on wait(timeout)', function()
+    local ret = exec_lua [[
+      return vim.system({ 'sleep', '10' }):wait(1000)
+    ]]
+    eq({
+      code = 124,
+      signal = 15, -- SIGTERM
+      stdout = '',
+      stderr = '',
+    }, ret)
+  end)
+
   it('kill processes', function()
     exec_lua([[
       local signal