To: vim_dev@googlegroups.com Subject: Patch 8.1.0350 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.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) Files: src/channel.c, src/structs.h, src/testdir/test_channel.vim, runtime/doc/channel.txt *** ../vim-8.1.0349/src/channel.c 2018-08-09 22:15:30.042856715 +0200 --- src/channel.c 2018-09-06 16:08:40.193230860 +0200 *************** *** 1180,1185 **** --- 1180,1186 ---- 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,3683 **** channel_set_nonblock(channel_T *channel, ch_part_T part) { chanpart_T *ch_part = &channel->ch_part[part]; ! int fd = ch_part->ch_fd; if (fd != INVALID_FD) { --- 3678,3684 ---- channel_set_nonblock(channel_T *channel, ch_part_T part) { chanpart_T *ch_part = &channel->ch_part[part]; ! int fd = ch_part->ch_fd; if (fd != INVALID_FD) { *************** *** 3722,3727 **** --- 3723,3731 ---- 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,4558 **** --- 4557,4568 ---- == 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) *** ../vim-8.1.0349/src/structs.h 2018-06-30 18:27:59.897025143 +0200 --- src/structs.h 2018-09-06 15:30:31.063357009 +0200 *************** *** 1651,1656 **** --- 1651,1657 ---- 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,1734 **** --- 1730,1736 ---- 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]; *** ../vim-8.1.0349/src/testdir/test_channel.vim 2018-04-28 21:07:37.000000000 +0200 --- src/testdir/test_channel.vim 2018-09-06 16:05:43.771212320 +0200 *************** *** 47,54 **** --- 47,57 ---- 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,458 **** call ch_log('Test_raw_pipe()') " Add a dummy close callback to avoid that messages are dropped when calling " ch_canread(). let job = job_start(s:python . " test_channel_pipe.py", ! \ {'mode': 'raw', 'drop': 'never'}) call assert_equal(v:t_job, type(job)) call assert_equal("run", job_status(job)) --- 454,462 ---- 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', 'noblock': 1}) call assert_equal(v:t_job, type(job)) call assert_equal("run", job_status(job)) *************** *** 1349,1354 **** --- 1353,1386 ---- 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("\", '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) *** ../vim-8.1.0349/runtime/doc/channel.txt 2018-06-03 14:42:17.824505143 +0200 --- runtime/doc/channel.txt 2018-09-06 16:14:07.501586039 +0200 *************** *** 163,168 **** --- 163,171 ---- 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,599 **** --- 597,613 ---- 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. *** ../vim-8.1.0349/src/version.c 2018-09-06 13:14:39.148722497 +0200 --- src/version.c 2018-09-06 16:25:42.981907095 +0200 *************** *** 796,797 **** --- 796,799 ---- { /* Add new patch number below this line */ + /**/ + 350, /**/ -- I have a drinking problem -- I can't afford it. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///