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.

diff --git a/src/channel.c b/src/channel.c
index 892c09ab7e42c382fab9ffa7a7393f4edec6227f..e1b0304c38a1461f57d2ba272e7845db8bbce1b8 100644 (file)
--- 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 d5114cba42c883cb3343403bf08b98871cc36138..a263c1c3d3bb12e44e2c985804eb2f32382cc72c 100644 (file)
--- 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 05df50af433598f29e7972b7e659fef54446a172..0dba83d8edeb3b625019a01ee25e1b0d4ed4b437 100644 (file)
--- 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 fa1a40f1315982915eb800062f0f126f007efe39..5f32506741bbaba903281604d5f084d31e5ba0d0 100644 (file)
--- 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 a5dfd2a04a5ba16db95655ef261b84f6ec67aa90..82b57c34b934ab47e2b2168a623a165d46b2fe4a 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -753,6 +753,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1787,
 /**/
     1786,
 /**/