jobstop/process_stop: send SIGTERM directly

This reverts the revert of #6644 (7c1a5d1d4), and handles it properly
now (with tests).

3 files changed, 59 insertions, 20 deletions

diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
index cad49e2007..c936583841 100644
--- a/src/nvim/event/process.c
+++ b/src/nvim/event/process.c
@@ -21,9 +21,9 @@
 # include "event/process.c.generated.h"
 #endif
 
-// Time (ns) for a process to exit cleanly before we send TERM/KILL.
-#define TERM_TIMEOUT 1000000000
-#define KILL_TIMEOUT (TERM_TIMEOUT * 2)
+// Time for a process to exit cleanly before we send KILL.
+// For pty processes SIGTERM is sent first (in case SIGHUP was not enough).
+#define KILL_TIMEOUT_MS 2000
 
 #define CLOSE_PROC_STREAM(proc, stream) \
   do { \
@@ -125,8 +125,6 @@ void process_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL
       // Close handles to process without killing it.
       CREATE_EVENT(loop->events, process_close_handles, 1, proc);
     } else {
-      uv_kill(proc->pid, SIGTERM);
-      proc->term_sent = true;
       process_stop(proc);
     }
   }
@@ -238,6 +236,8 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
       // stdout/stderr, they will be closed when it exits(possibly due to being
       // terminated after a timeout)
       process_close_in(proc);
+      ILOG("Sending SIGTERM to pid %d", proc->pid);
+      uv_kill(proc->pid, SIGTERM);
       break;
     case kProcessTypePty:
       // close all streams for pty processes to send SIGHUP to the process
@@ -251,9 +251,10 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
   Loop *loop = proc->loop;
   if (!loop->children_stop_requests++) {
     // When there's at least one stop request pending, start a timer that
-    // will periodically check if a signal should be send to a to the job
-    DLOG("Starting job kill timer");
-    uv_timer_start(&loop->children_kill_timer, children_kill_cb, 100, 100);
+    // will periodically check if a signal should be send to the job.
+    ILOG("Starting job kill timer");
+    uv_timer_start(&loop->children_kill_timer, children_kill_cb,
+                   KILL_TIMEOUT_MS, KILL_TIMEOUT_MS);
   }
 }
 
@@ -269,15 +270,15 @@ static void children_kill_cb(uv_timer_t *handle)
     if (!proc->stopped_time) {
       continue;
     }
-    uint64_t elapsed = now - proc->stopped_time;
-
-    if (!proc->term_sent && elapsed >= TERM_TIMEOUT) {
-      ILOG("Sending SIGTERM to pid %d", proc->pid);
-      uv_kill(proc->pid, SIGTERM);
-      proc->term_sent = true;
-    } else if (elapsed >= KILL_TIMEOUT) {
-      ILOG("Sending SIGKILL to pid %d", proc->pid);
-      uv_kill(proc->pid, SIGKILL);
+    uint64_t elapsed = (now - proc->stopped_time) / 1000000 + 1;
+
+    if (elapsed >= KILL_TIMEOUT_MS) {
+      int sig = proc->type == kProcessTypePty && elapsed < KILL_TIMEOUT_MS * 2
+                    ? SIGTERM
+                    : SIGKILL;
+      ILOG("Sending %s to pid %d", sig == SIGTERM ? "SIGTERM" : "SIGKILL",
+           proc->pid);
+      uv_kill(proc->pid, sig);
     }
   }
 }
diff --git a/src/nvim/event/process.h b/src/nvim/event/process.h
index 26d70a5e6d..5c00e8e7ec 100644
--- a/src/nvim/event/process.h
+++ b/src/nvim/event/process.h
@@ -26,7 +26,7 @@ struct process {
   Stream *in, *out, *err;
   process_exit_cb cb;
   internal_process_cb internal_exit_cb, internal_close_cb;
-  bool closed, term_sent, detach;
+  bool closed, detach;
   MultiQueue *events;
 };
 
@@ -48,7 +48,6 @@ static inline Process process_init(Loop *loop, ProcessType type, void *data)
     .err = NULL,
     .cb = NULL,
     .closed = false,
-    .term_sent = false,
     .internal_close_cb = NULL,
     .internal_exit_cb = NULL,
     .detach = false
diff --git a/test/functional/autocmd/termclose_spec.lua b/test/functional/autocmd/termclose_spec.lua
index d4beab22e4..8cc49c0d4c 100644
--- a/test/functional/autocmd/termclose_spec.lua
+++ b/test/functional/autocmd/termclose_spec.lua
@@ -14,13 +14,52 @@ describe('TermClose event', function()
     nvim('set_option', 'shellcmdflag', 'EXE')
   end)
 
-  it('triggers when terminal job ends', function()
+  it('triggers when fast-exiting terminal job stops', function()
     command('autocmd TermClose * let g:test_termclose = 23')
     command('terminal')
     command('call jobstop(b:terminal_job_id)')
     retry(nil, nil, function() eq(23, eval('g:test_termclose')) end)
   end)
 
+  it('triggers when long-running terminal job gets stopped', function()
+    nvim('set_option', 'shell', 'sh')
+    command('autocmd TermClose * let g:test_termclose = 23')
+    command('terminal')
+    command('call jobstop(b:terminal_job_id)')
+    retry(nil, nil, function() eq(23, eval('g:test_termclose')) end)
+  end)
+
+  it('kills job trapping SIGTERM', function()
+    nvim('set_option', 'shell', 'sh')
+    nvim('set_option', 'shellcmdflag', '-c')
+    command([[ let g:test_job = jobstart('trap "" TERM && echo 1 && sleep 60', { ]]
+      .. [[ 'on_stdout': {-> execute('let g:test_job_started = 1')}, ]]
+      .. [[ 'on_exit': {-> execute('let g:test_job_exited = 1')}}) ]])
+    retry(nil, nil, function() eq(1, eval('get(g:, "test_job_started", 0)')) end)
+
+    local start = os.time()
+    command('call jobstop(g:test_job)')
+    retry(nil, nil, function() eq(1, eval('get(g:, "test_job_exited", 0)')) end)
+    local duration = os.time() - start
+    eq(2, duration)
+  end)
+
+  it('kills pty job trapping SIGHUP and SIGTERM', function()
+    nvim('set_option', 'shell', 'sh')
+    nvim('set_option', 'shellcmdflag', '-c')
+    command([[ let g:test_job = jobstart('trap "" HUP TERM && echo 1 && sleep 60', { ]]
+      .. [[ 'pty': 1,]]
+      .. [[ 'on_stdout': {-> execute('let g:test_job_started = 1')}, ]]
+      .. [[ 'on_exit': {-> execute('let g:test_job_exited = 1')}}) ]])
+    retry(nil, nil, function() eq(1, eval('get(g:, "test_job_started", 0)')) end)
+
+    local start = os.time()
+    command('call jobstop(g:test_job)')
+    retry(nil, nil, function() eq(1, eval('get(g:, "test_job_exited", 0)')) end)
+    local duration = os.time() - start
+    eq(4, duration)
+  end)
+
   it('reports the correct <abuf>', function()
     command('set hidden')
     command('autocmd TermClose * let g:abuf = expand("<abuf>")')