From 6d5f12efd286c684de8608c07bb0f76a9d594b5b Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Mon, 4 Sep 2023 11:30:16 +0100 Subject: [PATCH] fix(vim.system): make timeout work properly Mimic the behaviour of timeout(1) from coreutils. --- runtime/doc/lua.txt | 10 ++- runtime/lua/vim/_editor.lua | 8 ++- runtime/lua/vim/_system.lua | 102 +++++++++++++++++----------- test/functional/lua/system_spec.lua | 42 ++++++++---- 4 files changed, 103 insertions(+), 59 deletions(-) diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt index f5b3e56f61..61aee559ef 100644 --- a/runtime/doc/lua.txt +++ b/runtime/doc/lua.txt @@ -1777,7 +1777,9 @@ vim.system({cmd}, {opts}, {on_exit}) *vim.system()* `fun(err: string, data: string)`. Defaults to `true`. • text: (boolean) Handle stdout and stderr as text. Replaces `\r\n` with `\n`. - • timeout: (integer) + • 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. • 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 @@ -1792,14 +1794,16 @@ vim.system({cmd}, {opts}, {on_exit}) *vim.system()* Return: ~ SystemObj Object with the fields: • pid (integer) Process ID - • wait (fun(timeout: integer|nil): SystemCompleted) + • 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. • SystemCompleted is an object with the fields: • code: (integer) • signal: (integer) • stdout: (string), nil if stdout argument is passed • stderr: (string), nil if stderr argument is passed - • kill (fun(signal: integer)) + • kill (fun(signal: integer|string)) • write (fun(data: string|nil)) Requires `stdin=true`. Pass `nil` to close the stream. • is_closing (fun(): boolean) diff --git a/runtime/lua/vim/_editor.lua b/runtime/lua/vim/_editor.lua index 96ac379368..ba0345cdd5 100644 --- a/runtime/lua/vim/_editor.lua +++ b/runtime/lua/vim/_editor.lua @@ -107,7 +107,8 @@ vim.log = { --- Handle output from stdout. 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) +--- - 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. --- - 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 @@ -118,13 +119,14 @@ vim.log = { --- --- @return SystemObj Object with the fields: --- - pid (integer) Process ID ---- - wait (fun(timeout: integer|nil): SystemCompleted) +--- - 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. --- - SystemCompleted is an object with the fields: --- - code: (integer) --- - signal: (integer) --- - stdout: (string), nil if stdout argument is passed --- - stderr: (string), nil if stderr argument is passed ---- - kill (fun(signal: integer)) +--- - kill (fun(signal: integer|string)) --- - write (fun(data: string|nil)) Requires `stdin=true`. Pass `nil` to close the stream. --- - is_closing (fun(): boolean) function vim.system(cmd, opts, on_exit) diff --git a/runtime/lua/vim/_system.lua b/runtime/lua/vim/_system.lua index ba3fc34e78..7bd7ff91f9 100644 --- a/runtime/lua/vim/_system.lua +++ b/runtime/lua/vim/_system.lua @@ -2,8 +2,8 @@ local uv = vim.uv --- @class SystemOpts --- @field stdin? string|string[]|true ---- @field stdout? fun(err:string, data: string)|false ---- @field stderr? fun(err:string, data: string)|false +--- @field stdout? fun(err:string?, data: string?)|false +--- @field stderr? fun(err:string?, data: string?)|false --- @field cwd? string --- @field env? table --- @field clear_env? boolean @@ -18,39 +18,46 @@ local uv = vim.uv --- @field stderr? string --- @class SystemState ---- @field cmd string[] --- @field handle? uv.uv_process_t --- @field timer? uv.uv_timer_t --- @field pid? integer --- @field timeout? integer ---- @field done? boolean +--- @field done? boolean|'timeout' --- @field stdin? uv.uv_stream_t --- @field stdout? uv.uv_stream_t --- @field stderr? uv.uv_stream_t --- @field result? SystemCompleted ----@param state SystemState -local function close_handles(state) - for _, handle in pairs({ state.handle, state.stdin, state.stdout, state.stderr }) do - if not handle:is_closing() then - handle:close() - end +--- @enum vim.SystemSig +local SIG = { + HUP = 1, -- Hangup + INT = 2, -- Interrupt from keyboard + KILL = 9, -- Kill signal + TERM = 15, -- Termination signal + -- STOP = 17,19,23 -- Stop the process +} + +---@param handle uv.uv_handle_t? +local function close_handle(handle) + if handle and not handle:is_closing() then + handle:close() end end ---- @param cmd string[] ---- @return SystemCompleted -local function timeout_result(cmd) - local cmd_str = table.concat(cmd, ' ') - local err = string.format("Command timed out: '%s'", cmd_str) - return { code = 0, signal = 2, stdout = '', stderr = err } +---@param state SystemState +local function close_handles(state) + close_handle(state.handle) + close_handle(state.stdin) + close_handle(state.stdout) + close_handle(state.stderr) + close_handle(state.timer) end --- @class SystemObj --- @field pid integer --- @field private _state SystemState --- @field wait fun(self: SystemObj, timeout?: integer): SystemCompleted ---- @field kill fun(self: SystemObj, signal: integer) +--- @field kill fun(self: SystemObj, signal: integer|string) --- @field write fun(self: SystemObj, data?: string|string[]) --- @field is_closing fun(self: SystemObj): boolean? local SystemObj = {} @@ -69,6 +76,13 @@ function SystemObj:kill(signal) self._state.handle:kill(signal) end +--- @package +--- @param signal? vim.SystemSig +function SystemObj:_timeout(signal) + self._state.done = 'timeout' + self:kill(signal or SIG.TERM) +end + local MAX_TIMEOUT = 2 ^ 31 --- @param timeout? integer @@ -76,13 +90,16 @@ local MAX_TIMEOUT = 2 ^ 31 function SystemObj:wait(timeout) local state = self._state - vim.wait(timeout or state.timeout or MAX_TIMEOUT, function() - return state.done + local done = vim.wait(timeout or state.timeout or MAX_TIMEOUT, function() + return state.result ~= nil end) - if not state.done then - self:kill(6) -- 'sigint' - state.result = timeout_result(state.cmd) + 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() + return state.result ~= nil + end) end return state.result @@ -124,9 +141,9 @@ function SystemObj:is_closing() return handle == nil or handle:is_closing() end ----@param output function|'false' +---@param output fun(err:string?, data: string?)|false ---@return uv.uv_stream_t? ----@return function? Handler +---@return fun(err:string?, data: string?)? Handler local function setup_output(output) if output == nil then return assert(uv.new_pipe(false)), nil @@ -224,6 +241,19 @@ local function spawn(cmd, opts, on_exit, on_error) return handle, pid_or_err --[[@as integer]] end +---@param timeout integer +---@param cb fun() +---@return uv.uv_timer_t +local function timer_oneshot(timeout, cb) + local timer = assert(uv.new_timer()) + timer:start(timeout, 0, function() + timer:stop() + timer:close() + cb() + end) + return timer +end + --- Run a system command --- --- @param cmd string[] @@ -267,10 +297,6 @@ function M.run(cmd, opts, on_exit) hide = true, }, function(code, signal) close_handles(state) - if state.timer then - state.timer:stop() - state.timer:close() - end local check = assert(uv.new_check()) @@ -283,7 +309,14 @@ function M.run(cmd, opts, on_exit) check:stop() check:close() - state.done = true + if state.done == nil then + state.done = true + end + + if code == 0 and state.done == 'timeout' then + code = 124 + end + state.result = { code = code, signal = signal, @@ -317,16 +350,9 @@ function M.run(cmd, opts, on_exit) end if opts.timeout then - state.timer = assert(uv.new_timer()) - state.timer:start(opts.timeout, 0, function() - state.timer:stop() - state.timer:close() + state.timer = timer_oneshot(opts.timeout, function() if state.handle and state.handle:is_active() then - obj:kill(6) --- 'sigint' - state.result = timeout_result(state.cmd) - if on_exit then - on_exit(state.result) - end + obj:_timeout() end end) end diff --git a/test/functional/lua/system_spec.lua b/test/functional/lua/system_spec.lua index 35b9d5cc37..9321468f84 100644 --- a/test/functional/lua/system_spec.lua +++ b/test/functional/lua/system_spec.lua @@ -5,27 +5,39 @@ local eq = helpers.eq local function system_sync(cmd, opts) return exec_lua([[ - return vim.system(...):wait() + local obj = vim.system(...) + local pid = obj.pid + local res = obj:wait() + + -- Check the process is no longer running + vim.fn.systemlist({'ps', 'p', tostring(pid)}) + assert(vim.v.shell_error == 1, 'process still exists') + + return res ]], cmd, opts) end local function system_async(cmd, opts) - exec_lua([[ + return exec_lua([[ local cmd, opts = ... _G.done = false - vim.system(cmd, opts, function(obj) + local obj = vim.system(cmd, opts, function(obj) _G.done = true _G.ret = obj end) + + local done = vim.wait(10000, function() + return _G.done + end) + + assert(done, 'process did not exit') + + -- Check the process is no longer running + vim.fn.systemlist({'ps', 'p', tostring(obj.pid)}) + assert(vim.v.shell_error == 1, 'process still exists') + + return _G.ret ]], cmd, opts) - - while true do - if exec_lua[[return _G.done]] then - break - end - end - - return exec_lua[[return _G.ret]] end describe('vim.system', function() @@ -43,12 +55,12 @@ describe('vim.system', function() eq('hellocat', system({ 'cat' }, { stdin = 'hellocat', text = true }).stdout) end) - it ('supports timeout', function() + it('supports timeout', function() eq({ - code = 0, - signal = 2, + code = 124, + signal = 15, stdout = '', - stderr = "Command timed out: 'sleep 10'" + stderr = '' }, system({ 'sleep', '10' }, { timeout = 1 })) end) end)