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.

Author: brammool

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;
 }
 

src/eval.c

@@ -7730,12 +7730,21 @@ get_dict_tv(char_u **arg, typval_T *rettv, int evaluate)
     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
 

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);

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

src/version.c

@@ -747,6 +747,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1322,
 /**/
     1321,
 /**/