patch 8.1.0350: Vim may block on ch_sendraw()

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)

diff --git a/runtime/doc/channel.txt b/runtime/doc/channel.txt
index 4cc36258c0c85cd4b5b98f43118edd25a0c3452d..fd08cd49742e8045ded6494be3afe33e3777c90f 100644 (file)
--- 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 282340eefc74ed2fe618b2e07b605760954135d2..793dbaa8336e50f0b742745246e2ffacc14cadc5 100644 (file)
--- 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 ec109eb893022ee9d10a2295a797cdfd825aca6f..253f774abc2b1ed599cf208a60c867fce3a7cc24 100644 (file)
--- 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 371afc79975650e8c9c3b69dc41d5e33e827da18..5d071473a5ad6118ba25674bbeb88fa612c64528 100644 (file)
--- 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 9e399a656df1391a326374dceeca97de5e8bcf9d..292af60f5340fffc3153d4c01e8c3f723c4cb208 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -794,6 +794,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    350,
 /**/
     349,
 /**/