patch 7.4.1322

Problem:    Crash when unletting the variable that holds the channel in a
            callback function.  (Christian Robinson)
Solution:   Increase the reference count while invoking the callback.

diff --git a/src/channel.c b/src/channel.c
index a806c5681a4b11b7b11e1e48042845a9d48ab023..cad5a1bc1f93e147fbd6f3f14075bd18bcfac4f9 100644 (file)
--- a/src/channel.c
+++ b/src/channel.c
@@ -1217,8 +1217,6 @@ channel_close(channel_T *channel)
 #endif
 
     channel->ch_close_cb = NULL;
-    vim_free(channel->ch_callback);
-    channel->ch_callback = NULL;
     channel_clear(channel);
 }
 
@@ -1298,6 +1296,9 @@ channel_clear(channel_T *channel)
        free_tv(json_head->jq_next->jq_value);
        remove_json_node(json_head, json_head->jq_next);
     }
+
+    vim_free(channel->ch_callback);
+    channel->ch_callback = NULL;
 }
 
 #if defined(EXITFREE) || defined(PROTO)
@@ -1802,15 +1803,28 @@ channel_select_check(int ret_in, void *rfds_in)
     int
 channel_parse_messages(void)
 {
-    channel_T *channel;
-    int            ret = FALSE;
+    channel_T  *channel = first_channel;
+    int                ret = FALSE;
+    int                r;
 
-    for (channel = first_channel; channel != NULL; channel = channel->ch_next)
-       while (may_invoke_callback(channel) == OK)
+    while (channel != NULL)
+    {
+       /* Increase the refcount, in case the handler causes the channel to be
+        * unreferenced or closed. */
+       ++channel->ch_refcount;
+       r = may_invoke_callback(channel);
+       if (channel_unref(channel))
+           /* channel was freed, start over */
+           channel = first_channel;
+
+       if (r == OK)
        {
-           channel = first_channel;  /* start over */
+           channel = first_channel;  /* something was done, start over */
            ret = TRUE;
        }
+       else
+           channel = channel->ch_next;
+    }
     return ret;
 }
 

diff --git a/src/eval.c b/src/eval.c
index a36f3dd51d0eef34c7a68ac12d2b4ea7ec4cab81..53d41d1d98dc72be45610e145842736c759cf477 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -7730,12 +7730,21 @@ failret:
     return OK;
 }
 
-#ifdef FEAT_CHANNEL
-    static void
+#if defined(FEAT_CHANNEL) || defined(PROTO)
+/*
+ * Decrement the reference count on "channel" and free it when it goes down to
+ * zero.
+ * Returns TRUE when the channel was freed.
+ */
+    int
 channel_unref(channel_T *channel)
 {
     if (channel != NULL && --channel->ch_refcount <= 0)
+    {
        channel_free(channel);
+       return TRUE;
+    }
+    return FALSE;
 }
 #endif
 

diff --git a/src/proto/eval.pro b/src/proto/eval.pro
index d1f5c3cc8e651ea031757e83a22f6b03ca33fd3a..e63205e11cbf33cbf2ae6795a5fff70dbb8a3257 100644 (file)
--- a/src/proto/eval.pro
+++ b/src/proto/eval.pro
@@ -79,6 +79,7 @@ int dict_add_list(dict_T *d, char *key, list_T *list);
 dictitem_T *dict_find(dict_T *d, char_u *key, int len);
 char_u *get_dict_string(dict_T *d, char_u *key, int save);
 long get_dict_number(dict_T *d, char_u *key);
+int channel_unref(channel_T *channel);
 int string2float(char_u *text, float_T *value);
 char_u *get_function_name(expand_T *xp, int idx);
 char_u *get_expr_name(expand_T *xp, int idx);

diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 0e87899f753c87e201bdaf8960b71beef0b9d673..5b7812182fc329990b188b21d12aa0e7058af39a 100644 (file)
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -295,3 +295,21 @@ func Test_pipe()
     call job_stop(job)
   endtry
 endfunc
+
+let s:unletResponse = ''
+func s:UnletHandler(handle, msg)
+  let s:unletResponse = a:msg
+  unlet s:channelfd
+endfunc
+
+" Test that "unlet handle" in a handler doesn't crash Vim.
+func s:unlet_handle(port)
+  let s:channelfd = ch_open('localhost:' . a:port, s:chopt)
+  call ch_sendexpr(s:channelfd, "test", function('s:UnletHandler'))
+  sleep 10m
+  call assert_equal('what?', s:unletResponse)
+endfunc
+
+func Test_unlet_handle()
+  call s:run_server('s:unlet_handle')
+endfunc

diff --git a/src/version.c b/src/version.c
index f8d838b44d98dddec6f4e6b1b13d9081242e9b39..f5da6fcdfc3ed6084f8023bd67de85ca3938a0bc 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -747,6 +747,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1322,
 /**/
     1321,
 /**/