Merge pull request #4760 from justinmk/term-use-after-free

term_close use-after-free

4 files changed, 28 insertions, 10 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 8c8881b398..712ee06b85 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -21701,6 +21701,18 @@ static void term_resize(uint16_t width, uint16_t height, void *d)
   pty_process_resize(&data->proc.pty, width, height);
 }
 
+static inline void term_delayed_free(void **argv)
+{
+  TerminalJobData *j = argv[0];
+  if (j->in.pending_reqs || j->out.pending_reqs || j->err.pending_reqs) {
+    queue_put(j->events, term_delayed_free, 1, j);
+    return;
+  }
+
+  terminal_destroy(j->term);
+  term_job_data_decref(j);
+}
+
 static void term_close(void *d)
 {
   TerminalJobData *data = d;
@@ -21708,8 +21720,7 @@ static void term_close(void *d)
     data->exited = true;
     process_stop((Process *)&data->proc);
   }
-  terminal_destroy(data->term);
-  term_job_data_decref(d);
+  queue_put(data->events, term_delayed_free, 1, data);
 }
 
 static void term_job_data_decref(TerminalJobData *data)
diff --git a/test/functional/legacy/068_text_formatting_spec.lua b/test/functional/legacy/068_text_formatting_spec.lua
index 0e3d66e0e5..cac8be77f3 100644
--- a/test/functional/legacy/068_text_formatting_spec.lua
+++ b/test/functional/legacy/068_text_formatting_spec.lua
@@ -1,7 +1,5 @@
--- Test for text formatting.
-
 local helpers = require('test.functional.helpers')
-local feed, insert, source = helpers.feed, helpers.insert, helpers.source
+local feed, insert = helpers.feed, helpers.insert
 local clear, execute, expect = helpers.clear, helpers.execute, helpers.expect
 
 describe('text formatting', function()
diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua
index 55ef254a63..cefb603a7e 100644
--- a/test/functional/terminal/buffer_spec.lua
+++ b/test/functional/terminal/buffer_spec.lua
@@ -158,8 +158,7 @@ describe('terminal buffer', function()
   end)
 
   it('handles loss of focus gracefully', function()
-    -- Temporarily change the statusline to avoid printing the file name, which
-    -- varies be where the test is run.
+    -- Change the statusline to avoid printing the file name, which varies.
     nvim('set_option', 'statusline', '==========')
     execute('set laststatus=0')
 
@@ -195,5 +194,15 @@ describe('terminal buffer', function()
 
     execute('set laststatus=1')  -- Restore laststatus to the default.
   end)
+
+  it('term_close() use-after-free #4393', function()
+    if eval("executable('yes')") == 0 then
+      pending('missing "yes" command')
+      return
+    end
+    execute('terminal yes')
+    feed([[<C-\><C-n>]])
+    execute('bdelete!')
+  end)
 end)
 
diff --git a/test/functional/terminal/ex_terminal_spec.lua b/test/functional/terminal/ex_terminal_spec.lua
index a3937ca4ba..d89092ff27 100644
--- a/test/functional/terminal/ex_terminal_spec.lua
+++ b/test/functional/terminal/ex_terminal_spec.lua
@@ -1,7 +1,7 @@
 local helpers = require('test.functional.helpers')
 local Screen = require('test.functional.ui.screen')
 local clear, wait, nvim = helpers.clear, helpers.wait, helpers.nvim
-local nvim_dir, source, ok = helpers.nvim_dir, helpers.source, helpers.ok
+local nvim_dir, source, eq = helpers.nvim_dir, helpers.source, helpers.eq
 local execute, eval = helpers.execute, helpers.eval
 
 describe(':terminal', function()
@@ -53,7 +53,7 @@ describe(':terminal', function()
     source([[
       autocmd BufNew * set shell=foo
       terminal]])
-      -- Verify that BufNew actually fired (else the test is useless).
-    ok('foo' == eval('&shell'))
+      -- Verify that BufNew actually fired (else the test is invalid).
+    eq('foo', eval('&shell'))
   end)
 end)