To: vim_dev@googlegroups.com Subject: Patch 7.4.1787 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 7.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. Files: src/channel.c, src/proto/channel.pro, src/testdir/test_channel.vim, src/testdir/test_channel_pipe.py *** ../vim-7.4.1786/src/channel.c 2016-04-18 19:27:18.022188461 +0200 --- src/channel.c 2016-04-26 17:15:17.372589946 +0200 *************** *** 54,59 **** --- 54,61 ---- # 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,2444 **** typval_T argv[1]; typval_T rettv; int dummy; ! /* 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; ++channel->ch_refcount; ! 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); --channel->ch_refcount; /* the callback is only called once */ --- 2429,2456 ---- typval_T argv[1]; typval_T rettv; int dummy; + int part; ! /* 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; ! 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); ! } --channel->ch_refcount; /* the callback is only called once */ *************** *** 2592,2602 **** } #endif /* * Check for reading from "fd" with "timeout" msec. ! * Return FAIL when there is nothing to read. */ ! static int channel_wait(channel_T *channel, sock_T fd, int timeout) { if (timeout > 0) --- 2604,2622 ---- } #endif + typedef enum { + CW_READY, + CW_NOT_READY, + CW_ERROR + } channel_wait_result; + /* * Check for reading from "fd" with "timeout" msec. ! * 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 channel_wait_result channel_wait(channel_T *channel, sock_T fd, int timeout) { if (timeout > 0) *************** *** 2613,2621 **** /* reading from a pipe, not a socket */ while (TRUE) { ! if (PeekNamedPipe((HANDLE)fd, NULL, 0, NULL, &nread, NULL) ! && nread > 0) ! return OK; /* perhaps write some buffer lines */ channel_write_any_lines(); --- 2633,2644 ---- /* reading from a pipe, not a socket */ while (TRUE) { ! 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,2671 **** if (ret > 0) { if (FD_ISSET(fd, &rfds)) ! return OK; channel_write_any_lines(); continue; } --- 2688,2694 ---- if (ret > 0) { if (FD_ISSET(fd, &rfds)) ! return CW_READY; channel_write_any_lines(); continue; } *************** *** 2683,2689 **** if (poll(fds, nfd, timeout) > 0) { if (fds[0].revents & POLLIN) ! return OK; channel_write_any_lines(); continue; } --- 2706,2712 ---- if (poll(fds, nfd, timeout) > 0) { if (fds[0].revents & POLLIN) ! return CW_READY; channel_write_any_lines(); continue; } *************** *** 2691,2697 **** } #endif } ! return FAIL; } /* --- 2714,2749 ---- } #endif } ! 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,2705 **** * "part" is PART_SOCK, PART_OUT or PART_ERR. * The data is put in the read queue. */ ! void channel_read(channel_T *channel, int part, char *func) { static char_u *buf = NULL; --- 2751,2757 ---- * "part" is PART_SOCK, PART_OUT or PART_ERR. * The data is put in the read queue. */ ! static void channel_read(channel_T *channel, int part, char *func) { static char_u *buf = NULL; *************** *** 2729,2735 **** * MAXMSGSIZE long. */ for (;;) { ! if (channel_wait(channel, fd, 0) == FAIL) break; if (use_socket) len = sock_read(fd, (char *)buf, MAXMSGSIZE); --- 2781,2787 ---- * MAXMSGSIZE long. */ for (;;) { ! if (channel_wait(channel, fd, 0) != CW_READY) break; if (use_socket) len = sock_read(fd, (char *)buf, MAXMSGSIZE); *************** *** 2747,2779 **** /* 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)(); ! } #if defined(CH_HAS_GUI) && defined(FEAT_GUI_GTK) /* signal the main loop that there is something to read */ --- 2799,2805 ---- /* Reading a disconnection (readlen == 0), or an error. */ if (readlen <= 0) ! 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,2818 **** /* Wait for up to the channel timeout. */ if (fd == INVALID_FD) return NULL; ! if (channel_wait(channel, fd, timeout) == FAIL) { ch_log(channel, "Timed out"); return NULL; --- 2838,2844 ---- /* Wait for up to the channel timeout. */ if (fd == INVALID_FD) return NULL; ! if (channel_wait(channel, fd, timeout) != CW_READY) { ch_log(channel, "Timed out"); return NULL; *************** *** 2916,2922 **** timeout = timeout_arg; } fd = chanpart->ch_fd; ! if (fd == INVALID_FD || channel_wait(channel, fd, timeout) == FAIL) { if (timeout == timeout_arg) { --- 2942,2949 ---- timeout = timeout_arg; } fd = chanpart->ch_fd; ! if (fd == INVALID_FD ! || channel_wait(channel, fd, timeout) != CW_READY) { if (timeout == timeout_arg) { *************** *** 3037,3044 **** 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"); } } } --- 3064,3079 ---- for (part = PART_SOCK; part <= PART_ERR; ++part) { fd = channel->ch_part[part].ch_fd; ! 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()"); ! } } } } *** ../vim-7.4.1786/src/proto/channel.pro 2016-04-08 17:07:09.546160667 +0200 --- src/proto/channel.pro 2016-04-26 16:20:47.619609183 +0200 *************** *** 26,32 **** 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); --- 26,31 ---- *** ../vim-7.4.1786/src/testdir/test_channel.vim 2016-04-14 12:46:33.620678603 +0200 --- src/testdir/test_channel.vim 2016-04-26 17:13:03.886037266 +0200 *************** *** 1048,1053 **** --- 1048,1085 ---- 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 = '' *** ../vim-7.4.1786/src/testdir/test_channel_pipe.py 2016-03-08 18:27:16.873343434 +0100 --- src/testdir/test_channel_pipe.py 2016-04-26 11:46:15.601576818 +0200 *************** *** 16,21 **** --- 16,23 ---- else: print(sys.argv[1]) sys.stdout.flush() + if sys.argv[1].startswith("quit"): + sys.exit(0) while True: typed = sys.stdin.readline() *** ../vim-7.4.1786/src/version.c 2016-04-24 15:41:29.033170205 +0200 --- src/version.c 2016-04-25 23:17:46.368755761 +0200 *************** *** 755,756 **** --- 755,758 ---- { /* Add new patch number below this line */ + /**/ + 1787, /**/ -- ARTHUR: Did you say shrubberies? ROGER: Yes. Shrubberies are my trade. I am a shrubber. My name is Roger the Shrubber. I arrange, design, and sell shrubberies. "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// 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 ///