eval: Protect job callbacks from being redefined

ref: #3188

4 files changed, 51 insertions, 4 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index a5ab57785c..006601211e 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -18987,7 +18987,7 @@ void ex_function(exarg_T *eap)
         emsg_funcname(e_funcexts, name);
         goto erret;
       }
-      if (fp->uf_calls > 0) {
+      if (fp->uf_refcount > 1 || fp->uf_calls > 0) {
         emsg_funcname(N_("E127: Cannot redefine function %s: It is in use"),
             name);
         goto erret;
@@ -21136,14 +21136,18 @@ static inline bool common_job_start(TerminalJobData *data, typval_T *rettv)
 {
   data->refcount++;
   Process *proc = (Process *)&data->proc;
+  char *cmd = xstrdup(proc->argv[0]);
   if (!process_spawn(proc)) {
-    EMSG(_(e_jobexe));
+    EMSG2(_(e_jobspawn), cmd);
+    xfree(cmd);
     if (proc->type == kProcessTypePty) {
       xfree(data->proc.pty.term_name);
-      free_term_job_data(data);
     }
+    rettv->vval.v_number = proc->status;
+    term_job_data_decref(data);
     return false;
   }
+  xfree(cmd);
 
   data->id = current_job_id++;
   wstream_init(proc->in, 0);
diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
index 81d4e690c3..bacbf4f8c7 100644
--- a/src/nvim/event/process.c
+++ b/src/nvim/event/process.c
@@ -66,7 +66,12 @@ bool process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
     if (proc->err) {
       uv_close((uv_handle_t *)&proc->err->uv.pipe, NULL);
     }
-    process_close(proc);
+
+    if (proc->type == kProcessTypeUv) {
+      uv_close((uv_handle_t *)&(((UvProcess *)proc)->uv), NULL);
+    } else {
+      process_close(proc);
+    }
     shell_free_argv(proc->argv);
     proc->status = -1;
     return false;
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index 68cb923e42..5126195841 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -1107,6 +1107,8 @@ EXTERN char_u e_isadir2[] INIT(= N_("E17: \"%s\" is a directory"));
 EXTERN char_u e_invjob[] INIT(= N_("E900: Invalid job id"));
 EXTERN char_u e_jobtblfull[] INIT(= N_("E901: Job table is full"));
 EXTERN char_u e_jobexe[] INIT(= N_("E902: \"%s\" is not an executable"));
+EXTERN char_u e_jobspawn[] INIT(= N_(
+      "E903: Process for command \"%s\" could not be spawned"));
 EXTERN char_u e_jobnotpty[] INIT(= N_("E904: Job is not connected to a pty"));
 EXTERN char_u e_libcall[] INIT(= N_("E364: Library call failed for \"%s()\""));
 EXTERN char_u e_mkdir[] INIT(= N_("E739: Cannot create directory %s: %s"));
diff --git a/test/functional/job/job_spec.lua b/test/functional/job/job_spec.lua
index 7085d61cc7..bdaf2a7ec6 100644
--- a/test/functional/job/job_spec.lua
+++ b/test/functional/job/job_spec.lua
@@ -6,6 +6,7 @@ local clear, eq, eval, execute, expect, feed, insert, neq, next_msg, nvim,
   helpers.insert, helpers.neq, helpers.next_message, helpers.nvim,
   helpers.nvim_dir, helpers.ok, helpers.run, helpers.session, helpers.source,
   helpers.stop, helpers.wait, helpers.write_file
+local Screen = require('test.functional.ui.screen')
 
 
 describe('jobs', function()
@@ -188,6 +189,41 @@ describe('jobs', function()
     eq({'notification', 'exit', {45, 10}}, next_msg())
   end)
 
+  it('cant redefine callbacks being used by a job', function()
+    local screen = Screen.new()
+    screen:attach()
+    local script = [[
+      function! g:JobHandler(job_id, data, event)
+      endfunction
+
+      let g:callbacks = {
+      \ 'on_stdout': function('g:JobHandler'),
+      \ 'on_stderr': function('g:JobHandler'),
+      \ 'on_exit': function('g:JobHandler')
+      \ }
+      let job = jobstart('cat -', g:callbacks)
+    ]]
+    source(script)
+    feed(':function! g:JobHandler(job_id, data, event)<cr>')
+    feed(':endfunction<cr>')
+    screen:expect([[
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      ~                                                    |
+      :function! g:JobHandler(job_id, data, event)         |
+      :  :endfunction                                      |
+      E127: Cannot redefine function JobHandler: It is in u|
+      se                                                   |
+      Press ENTER or type command to continue^              |
+    ]])
+  end)
+
   describe('jobwait', function()
     it('returns a list of status codes', function()
       source([[