patch 8.1.0350: Vim may block on ch_sendraw()v8.1.0350

Problem:    Vim may block on ch_sendraw() when the job is sending data back to
            Vim, which isn't read yet. (Nate Bosch)
Solution:   Add the "noblock" option to job_start(). (closes #2548)

5 files changed, 62 insertions, 2 deletions

diff --git a/runtime/doc/channel.txt b/runtime/doc/channel.txt
index 4cc36258c..fd08cd497 100644
--- a/runtime/doc/channel.txt
+++ b/runtime/doc/channel.txt
@@ -163,6 +163,9 @@ Use |ch_status()| to see if the channel could be opened.
 				The "close_cb" is also considered for this.
 		    "never"	All messages will be kept.
 
+							*channel-noblock*
+"noblock"	Same effect as |job-noblock|.  Only matters for writing.
+
 							*waittime*
 "waittime"	The time to wait for the connection to be made in
 		milliseconds.  A negative number waits forever.
@@ -594,6 +597,17 @@ See |job_setoptions()| and |ch_setoptions()|.
 			Note: when writing to a file or buffer and when
 			reading from a buffer NL mode is used by default.
 
+						*job-noblock*
+"noblock": 1		When writing use a non-blocking write call.  This
+			avoids getting stuck if Vim should handle other
+			messages in between, e.g. when a job sends back data
+			to Vim.  It implies that when `ch_sendraw()` returns
+			not all data may have been written yet.
+			This option was added in patch 8.1.0350, test with: >
+				if has("patch-8.1.350")
+				  let options['noblock'] = 1
+				endif
+<
 						*job-callback*
 "callback": handler	Callback for something to read on any part of the
 			channel.
diff --git a/src/channel.c b/src/channel.c
index 282340eef..793dbaa83 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -1180,6 +1180,7 @@ channel_set_options(channel_T *channel, jobopt_T *opt)
 	channel->ch_part[PART_OUT].ch_mode = opt->jo_out_mode;
     if (opt->jo_set & JO_ERR_MODE)
 	channel->ch_part[PART_ERR].ch_mode = opt->jo_err_mode;
+    channel->ch_nonblock = opt->jo_noblock;
 
     if (opt->jo_set & JO_TIMEOUT)
 	for (part = PART_SOCK; part < PART_COUNT; ++part)
@@ -3677,7 +3678,7 @@ channel_any_keep_open()
 channel_set_nonblock(channel_T *channel, ch_part_T part)
 {
     chanpart_T *ch_part = &channel->ch_part[part];
-    int fd = ch_part->ch_fd;
+    int		fd = ch_part->ch_fd;
 
     if (fd != INVALID_FD)
     {
@@ -3722,6 +3723,9 @@ channel_send(
 	return FAIL;
     }
 
+    if (channel->ch_nonblock && !ch_part->ch_nonblocking)
+	channel_set_nonblock(channel, part);
+
     if (ch_log_active())
     {
 	ch_log_lead("SEND ", channel, part);
@@ -4553,6 +4557,12 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 								      == FAIL)
 		    return FAIL;
 	    }
+	    else if (STRCMP(hi->hi_key, "noblock") == 0)
+	    {
+		if (!(supported & JO_MODE))
+		    break;
+		opt->jo_noblock = get_tv_number(item);
+	    }
 	    else if (STRCMP(hi->hi_key, "in_io") == 0
 		    || STRCMP(hi->hi_key, "out_io") == 0
 		    || STRCMP(hi->hi_key, "err_io") == 0)
diff --git a/src/structs.h b/src/structs.h
index ec109eb89..253f774ab 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1651,6 +1651,7 @@ struct channel_S {
     partial_T	*ch_close_partial;
     int		ch_drop_never;
     int		ch_keep_open;	/* do not close on read error */
+    int		ch_nonblock;
 
     job_T	*ch_job;	/* Job that uses this channel; this does not
 				 * count as a reference to avoid a circular
@@ -1729,6 +1730,7 @@ typedef struct
     ch_mode_T	jo_in_mode;
     ch_mode_T	jo_out_mode;
     ch_mode_T	jo_err_mode;
+    int		jo_noblock;
 
     job_io_T	jo_io[4];	/* PART_OUT, PART_ERR, PART_IN */
     char_u	jo_io_name_buf[4][NUMBUFLEN];
diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 371afc799..5d071473a 100644
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -47,8 +47,11 @@ endfunc
 func Ch_communicate(port)
   " Avoid dropping messages, since we don't use a callback here.
   let s:chopt.drop = 'never'
+  " Also add the noblock flag to try it out.
+  let s:chopt.noblock = 1
   let handle = ch_open('localhost:' . a:port, s:chopt)
   unlet s:chopt.drop
+  unlet s:chopt.noblock
   if ch_status(handle) == "fail"
     call assert_report("Can't open channel")
     return
@@ -451,8 +454,9 @@ func Test_raw_pipe()
   call ch_log('Test_raw_pipe()')
   " Add a dummy close callback to avoid that messages are dropped when calling
   " ch_canread().
+  " Also test the non-blocking option.
   let job = job_start(s:python . " test_channel_pipe.py",
-	\ {'mode': 'raw', 'drop': 'never'})
+	\ {'mode': 'raw', 'drop': 'never', 'noblock': 1})
   call assert_equal(v:t_job, type(job))
   call assert_equal("run", job_status(job))
 
@@ -1350,6 +1354,34 @@ endfunc
 
 """"""""""
 
+function ExitCbWipe(job, status)
+  exe g:wipe_buf 'bw!'
+endfunction
+
+" This caused a crash, because messages were handled while peeking for a
+" character.
+func Test_exit_cb_wipes_buf()
+  if !has('timers')
+    return
+  endif
+  set cursorline lazyredraw
+  call test_override('redraw_flag', 1)
+  new
+  let g:wipe_buf = bufnr('')
+
+  let job = job_start(['true'], {'exit_cb': 'ExitCbWipe'})
+  let timer = timer_start(300, {-> feedkeys("\<Esc>", 'nt')}, {'repeat': 5})
+  call feedkeys(repeat('g', 1000) . 'o', 'ntx!')
+  call WaitForAssert({-> assert_equal("dead", job_status(job))})
+  call timer_stop(timer)
+
+  set nocursorline nolazyredraw
+  unlet g:wipe_buf
+  call test_override('ALL', 0)
+endfunc
+
+""""""""""
+
 let g:Ch_unletResponse = ''
 func s:UnletHandler(handle, msg)
   let g:Ch_unletResponse = a:msg
diff --git a/src/version.c b/src/version.c
index 9e399a656..292af60f5 100644
--- a/src/version.c
+++ b/src/version.c
@@ -795,6 +795,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    350,
+/**/
     349,
 /**/
     348,