diff options
author | Bram Moolenaar <Bram@vim.org> | 2016-04-26 17:16:24 +0200 |
---|---|---|
committer | Bram Moolenaar <Bram@vim.org> | 2016-04-26 17:16:24 +0200 |
commit | b2658a1ab02cd0ba848164f70c7c464fdc398162 (patch) | |
tree | a84b975826fc11bbba0f8320d12e726f238be9b6 /src | |
parent | d10abe52019d10403eb559ea0a424bbd310b738f (diff) |
patch 7.4.1787v7.4.1787
Problem: When a job ends the close callback is invoked before other
callbacks. On Windows the close callback is not called.
Solution: First invoke out/err callbacks before the close callback.
Make the close callback work on Windows.
Diffstat (limited to 'src')
-rw-r--r-- | src/channel.c | 133 | ||||
-rw-r--r-- | src/proto/channel.pro | 1 | ||||
-rw-r--r-- | src/testdir/test_channel.vim | 32 | ||||
-rw-r--r-- | src/testdir/test_channel_pipe.py | 2 | ||||
-rw-r--r-- | src/version.c | 2 |
5 files changed, 120 insertions, 50 deletions
diff --git a/src/channel.c b/src/channel.c index 892c09ab7e..e1b0304c38 100644 --- a/src/channel.c +++ b/src/channel.c @@ -54,6 +54,8 @@ # define fd_close(sd) close(sd) #endif +static void channel_read(channel_T *channel, int part, char *func); + /* Whether a redraw is needed for appending a line to a buffer. */ static int channel_need_redraw = FALSE; @@ -2427,18 +2429,28 @@ channel_close(channel_T *channel, int invoke_close_cb) typval_T argv[1]; typval_T rettv; int dummy; + int part; - /* invoke the close callback; increment the refcount to avoid it - * being freed halfway */ - ch_logs(channel, "Invoking close callback %s", - (char *)channel->ch_close_cb); - argv[0].v_type = VAR_CHANNEL; - argv[0].vval.v_channel = channel; + /* Invoke callbacks before the close callback, since it's weird to + * first invoke the close callback. Increment the refcount to avoid + * the channel being freed halfway. */ ++channel->ch_refcount; - call_func(channel->ch_close_cb, (int)STRLEN(channel->ch_close_cb), + for (part = PART_SOCK; part <= PART_ERR; ++part) + while (may_invoke_callback(channel, part)) + ; + + /* Invoke the close callback, if still set. */ + if (channel->ch_close_cb != NULL) + { + ch_logs(channel, "Invoking close callback %s", + (char *)channel->ch_close_cb); + argv[0].v_type = VAR_CHANNEL; + argv[0].vval.v_channel = channel; + call_func(channel->ch_close_cb, (int)STRLEN(channel->ch_close_cb), &rettv, 1, argv, 0L, 0L, &dummy, TRUE, channel->ch_close_partial, NULL); - clear_tv(&rettv); + clear_tv(&rettv); + } --channel->ch_refcount; /* the callback is only called once */ @@ -2592,11 +2604,19 @@ channel_fill_poll_write(int nfd_in, struct pollfd *fds) } #endif +typedef enum { + CW_READY, + CW_NOT_READY, + CW_ERROR +} channel_wait_result; + /* * Check for reading from "fd" with "timeout" msec. - * Return FAIL when there is nothing to read. + * Return CW_READY when there is something to read. + * Return CW_NOT_READY when there is nothing to read. + * Return CW_ERROR when there is an error. */ - static int + static channel_wait_result channel_wait(channel_T *channel, sock_T fd, int timeout) { if (timeout > 0) @@ -2613,9 +2633,12 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) /* reading from a pipe, not a socket */ while (TRUE) { - if (PeekNamedPipe((HANDLE)fd, NULL, 0, NULL, &nread, NULL) - && nread > 0) - return OK; + int r = PeekNamedPipe((HANDLE)fd, NULL, 0, NULL, &nread, NULL); + + if (r && nread > 0) + return CW_READY; + if (r == 0) + return CW_ERROR; /* perhaps write some buffer lines */ channel_write_any_lines(); @@ -2665,7 +2688,7 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) if (ret > 0) { if (FD_ISSET(fd, &rfds)) - return OK; + return CW_READY; channel_write_any_lines(); continue; } @@ -2683,7 +2706,7 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) if (poll(fds, nfd, timeout) > 0) { if (fds[0].revents & POLLIN) - return OK; + return CW_READY; channel_write_any_lines(); continue; } @@ -2691,7 +2714,36 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) } #endif } - return FAIL; + return CW_NOT_READY; +} + + static void +channel_close_on_error(channel_T *channel, int part, char *func) +{ + /* Do not call emsg(), most likely the other end just exited. */ + ch_errors(channel, "%s(): Cannot read from channel", func); + + /* Queue a "DETACH" netbeans message in the command queue in order to + * terminate the netbeans session later. Do not end the session here + * directly as we may be running in the context of a call to + * netbeans_parse_messages(): + * netbeans_parse_messages + * -> autocmd triggered while processing the netbeans cmd + * -> ui_breakcheck + * -> gui event loop or select loop + * -> channel_read() + * Don't send "DETACH" for a JS or JSON channel. + */ + if (channel->ch_part[part].ch_mode == MODE_RAW + || channel->ch_part[part].ch_mode == MODE_NL) + channel_save(channel, part, (char_u *)DETACH_MSG_RAW, + (int)STRLEN(DETACH_MSG_RAW), FALSE, "PUT "); + + /* When reading from stdout is not possible, assume the other side has + * died. */ + channel_close(channel, TRUE); + if (channel->ch_nb_close_cb != NULL) + (*channel->ch_nb_close_cb)(); } /* @@ -2699,7 +2751,7 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) * "part" is PART_SOCK, PART_OUT or PART_ERR. * The data is put in the read queue. */ - void + static void channel_read(channel_T *channel, int part, char *func) { static char_u *buf = NULL; @@ -2729,7 +2781,7 @@ channel_read(channel_T *channel, int part, char *func) * MAXMSGSIZE long. */ for (;;) { - if (channel_wait(channel, fd, 0) == FAIL) + if (channel_wait(channel, fd, 0) != CW_READY) break; if (use_socket) len = sock_read(fd, (char *)buf, MAXMSGSIZE); @@ -2747,33 +2799,7 @@ channel_read(channel_T *channel, int part, char *func) /* Reading a disconnection (readlen == 0), or an error. */ if (readlen <= 0) - { - /* Do not give an error message, most likely the other end just - * exited. */ - ch_errors(channel, "%s(): Cannot read from channel", func); - - /* Queue a "DETACH" netbeans message in the command queue in order to - * terminate the netbeans session later. Do not end the session here - * directly as we may be running in the context of a call to - * netbeans_parse_messages(): - * netbeans_parse_messages - * -> autocmd triggered while processing the netbeans cmd - * -> ui_breakcheck - * -> gui event loop or select loop - * -> channel_read() - * Don't send "DETACH" for a JS or JSON channel. - */ - if (channel->ch_part[part].ch_mode == MODE_RAW - || channel->ch_part[part].ch_mode == MODE_NL) - channel_save(channel, part, (char_u *)DETACH_MSG_RAW, - (int)STRLEN(DETACH_MSG_RAW), FALSE, "PUT "); - - /* When reading from stdout is not possible, assume the other side has - * died. */ - channel_close(channel, TRUE); - if (channel->ch_nb_close_cb != NULL) - (*channel->ch_nb_close_cb)(); - } + channel_close_on_error(channel, part, func); #if defined(CH_HAS_GUI) && defined(FEAT_GUI_GTK) /* signal the main loop that there is something to read */ @@ -2812,7 +2838,7 @@ channel_read_block(channel_T *channel, int part, int timeout) /* Wait for up to the channel timeout. */ if (fd == INVALID_FD) return NULL; - if (channel_wait(channel, fd, timeout) == FAIL) + if (channel_wait(channel, fd, timeout) != CW_READY) { ch_log(channel, "Timed out"); return NULL; @@ -2916,7 +2942,8 @@ channel_read_json_block( timeout = timeout_arg; } fd = chanpart->ch_fd; - if (fd == INVALID_FD || channel_wait(channel, fd, timeout) == FAIL) + if (fd == INVALID_FD + || channel_wait(channel, fd, timeout) != CW_READY) { if (timeout == timeout_arg) { @@ -3037,8 +3064,16 @@ channel_handle_events(void) for (part = PART_SOCK; part <= PART_ERR; ++part) { fd = channel->ch_part[part].ch_fd; - if (fd != INVALID_FD && channel_wait(channel, fd, 0) == OK) - channel_read(channel, part, "channel_handle_events"); + if (fd != INVALID_FD) + { + int r = channel_wait(channel, fd, 0); + + if (r == CW_READY) + channel_read(channel, part, "channel_handle_events"); + else if (r == CW_ERROR) + channel_close_on_error(channel, part, + "channel_handle_events()"); + } } } } diff --git a/src/proto/channel.pro b/src/proto/channel.pro index d5114cba42..a263c1c3d3 100644 --- a/src/proto/channel.pro +++ b/src/proto/channel.pro @@ -26,7 +26,6 @@ void channel_close(channel_T *channel, int invoke_close_cb); char_u *channel_peek(channel_T *channel, int part); void channel_clear(channel_T *channel); void channel_free_all(void); -void channel_read(channel_T *channel, int part, char *func); char_u *channel_read_block(channel_T *channel, int part, int timeout); int channel_read_json_block(channel_T *channel, int part, int timeout_arg, int id, typval_T **rettv); void common_channel_read(typval_T *argvars, typval_T *rettv, int raw); diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim index 05df50af43..0dba83d8ed 100644 --- a/src/testdir/test_channel.vim +++ b/src/testdir/test_channel.vim @@ -1048,6 +1048,38 @@ func Test_out_cb() endtry endfunc +func Test_out_close_cb() + if !has('job') + return + endif + call ch_log('Test_out_close_cb()') + + let s:counter = 1 + let s:outmsg = 0 + let s:closemsg = 0 + func! OutHandler(chan, msg) + let s:outmsg = s:counter + let s:counter += 1 + endfunc + func! CloseHandler(chan) + let s:closemsg = s:counter + let s:counter += 1 + endfunc + let job = job_start(s:python . " test_channel_pipe.py quit now", + \ {'out_cb': 'OutHandler', + \ 'close_cb': 'CloseHandler'}) + call assert_equal("run", job_status(job)) + try + call s:waitFor('s:closemsg != 0 && s:outmsg != 0') + call assert_equal(1, s:outmsg) + call assert_equal(2, s:closemsg) + finally + call job_stop(job) + delfunc OutHandler + delfunc CloseHandler + endtry +endfunc + """""""""" let s:unletResponse = '' diff --git a/src/testdir/test_channel_pipe.py b/src/testdir/test_channel_pipe.py index fa1a40f131..5f32506741 100644 --- a/src/testdir/test_channel_pipe.py +++ b/src/testdir/test_channel_pipe.py @@ -16,6 +16,8 @@ if __name__ == "__main__": else: print(sys.argv[1]) sys.stdout.flush() + if sys.argv[1].startswith("quit"): + sys.exit(0) while True: typed = sys.stdin.readline() diff --git a/src/version.c b/src/version.c index a5dfd2a04a..82b57c34b9 100644 --- a/src/version.c +++ b/src/version.c @@ -754,6 +754,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 1787, +/**/ 1786, /**/ 1785, |