patch 7.4.1717

Problem:    Leaking memory when opening a channel fails.
Solution:   Unreference partials in job options.

diff --git a/src/channel.c b/src/channel.c
index b21c4432cc2ed14761d77e72c888f885ff6b582c..7b811dbce2b0491ef830e8ec918902e2d1954f13 100644 (file)
--- a/src/channel.c
+++ b/src/channel.c
@@ -858,7 +858,7 @@ channel_open_func(typval_T *argvars)
     char       *rest;
     int                port;
     jobopt_T    opt;
-    channel_T  *channel;
+    channel_T  *channel = NULL;
 
     address = get_tv_string(&argvars[0]);
     if (argvars[1].v_type != VAR_UNKNOWN
@@ -890,11 +890,11 @@ channel_open_func(typval_T *argvars)
     opt.jo_timeout = 2000;
     if (get_job_options(&argvars[1], &opt,
              JO_MODE_ALL + JO_CB_ALL + JO_WAITTIME + JO_TIMEOUT_ALL) == FAIL)
-       return NULL;
+       goto theend;
     if (opt.jo_timeout < 0)
     {
        EMSG(_(e_invarg));
-       return NULL;
+       goto theend;
     }
 
     channel = channel_open((char *)address, port, opt.jo_waittime, NULL);
@@ -903,6 +903,8 @@ channel_open_func(typval_T *argvars)
        opt.jo_set = JO_ALL;
        channel_set_options(channel, &opt);
     }
+theend:
+    free_job_options(&opt);
     return channel;
 }
 
@@ -2897,7 +2899,7 @@ common_channel_read(typval_T *argvars, typval_T *rettv, int raw)
     clear_job_options(&opt);
     if (get_job_options(&argvars[1], &opt, JO_TIMEOUT + JO_PART + JO_ID)
                                                                      == FAIL)
-       return;
+       goto theend;
 
     channel = get_channel_arg(&argvars[0], TRUE);
     if (channel != NULL)
@@ -2930,6 +2932,9 @@ common_channel_read(typval_T *argvars, typval_T *rettv, int raw)
            }
        }
     }
+
+theend:
+    free_job_options(&opt);
 }
 
 # if defined(WIN32) || defined(FEAT_GUI_X11) || defined(FEAT_GUI_GTK) \
@@ -3056,13 +3061,13 @@ send_common(
     channel_T  *channel;
     int                part_send;
 
+    clear_job_options(opt);
     channel = get_channel_arg(&argvars[0], TRUE);
     if (channel == NULL)
        return NULL;
     part_send = channel_part_send(channel);
     *part_read = channel_part_read(channel);
 
-    clear_job_options(opt);
     if (get_job_options(&argvars[2], opt, JO_CALLBACK + JO_TIMEOUT) == FAIL)
        return NULL;
 
@@ -3145,6 +3150,7 @@ ch_expr_common(typval_T *argvars, typval_T *rettv, int eval)
            free_tv(listtv);
        }
     }
+    free_job_options(&opt);
 }
 
 /*
@@ -3175,6 +3181,7 @@ ch_raw_common(typval_T *argvars, typval_T *rettv, int eval)
            timeout = channel_get_timeout(channel, part_read);
        rettv->vval.v_string = channel_read_block(channel, part_read, timeout);
     }
+    free_job_options(&opt);
 }
 
 # if (defined(UNIX) && !defined(HAVE_SELECT)) || defined(PROTO)
@@ -3545,12 +3552,31 @@ handle_io(typval_T *item, int part, jobopt_T *opt)
     return OK;
 }
 
+/*
+ * Clear a jobopt_T before using it.
+ */
     void
 clear_job_options(jobopt_T *opt)
 {
     vim_memset(opt, 0, sizeof(jobopt_T));
 }
 
+/*
+ * Free any members of a jobopt_T.
+ */
+    void
+free_job_options(jobopt_T *opt)
+{
+    if (opt->jo_partial != NULL)
+       partial_unref(opt->jo_partial);
+    if (opt->jo_out_partial != NULL)
+       partial_unref(opt->jo_out_partial);
+    if (opt->jo_err_partial != NULL)
+       partial_unref(opt->jo_err_partial);
+    if (opt->jo_close_partial != NULL)
+       partial_unref(opt->jo_close_partial);
+}
+
 /*
  * Get the PART_ number from the first character of an option name.
  */
@@ -4053,6 +4079,9 @@ job_start(typval_T *argvars)
        return NULL;
 
     job->jv_status = JOB_FAILED;
+#ifndef USE_ARGV
+    ga_init2(&ga, (int)sizeof(char*), 20);
+#endif
 
     /* Default mode is NL. */
     clear_job_options(&opt);
@@ -4060,7 +4089,7 @@ job_start(typval_T *argvars)
     if (get_job_options(&argvars[1], &opt,
            JO_MODE_ALL + JO_CB_ALL + JO_TIMEOUT_ALL + JO_STOPONEXIT
                           + JO_EXIT_CB + JO_OUT_IO + JO_BLOCK_WRITE) == FAIL)
-       return job;
+       goto theend;
 
     /* Check that when io is "file" that there is a file name. */
     for (part = PART_OUT; part <= PART_IN; ++part)
@@ -4070,7 +4099,7 @@ job_start(typval_T *argvars)
                    || *opt.jo_io_name[part] == NUL))
        {
            EMSG(_("E920: _io file requires _name to be set"));
-           return job;
+           goto theend;
        }
 
     if ((opt.jo_set & JO_IN_IO) && opt.jo_io[PART_IN] == JIO_BUFFER)
@@ -4091,7 +4120,7 @@ job_start(typval_T *argvars)
        else
            buf = buflist_find_by_name(opt.jo_io_name[PART_IN], FALSE);
        if (buf == NULL)
-           return job;
+           goto theend;
        if (buf->b_ml.ml_mfp == NULL)
        {
            char_u      numbuf[NUMBUFLEN];
@@ -4105,17 +4134,13 @@ job_start(typval_T *argvars)
            else
                s = opt.jo_io_name[PART_IN];
            EMSG2(_("E918: buffer must be loaded: %s"), s);
-           return job;
+           goto theend;
        }
        job->jv_in_buf = buf;
     }
 
     job_set_options(job, &opt);
 
-#ifndef USE_ARGV
-    ga_init2(&ga, (int)sizeof(char*), 20);
-#endif
-
     if (argvars[0].v_type == VAR_STRING)
     {
        /* Command is a string. */
@@ -4123,11 +4148,11 @@ job_start(typval_T *argvars)
        if (cmd == NULL || *cmd == NUL)
        {
            EMSG(_(e_invarg));
-           return job;
+           goto theend;
        }
 #ifdef USE_ARGV
        if (mch_parse_cmd(cmd, FALSE, &argv, &argc) == FAIL)
-           return job;
+           goto theend;
        argv[argc] = NULL;
 #endif
     }
@@ -4136,7 +4161,7 @@ job_start(typval_T *argvars)
            || argvars[0].vval.v_list->lv_len < 1)
     {
        EMSG(_(e_invarg));
-       return job;
+       goto theend;
     }
     else
     {
@@ -4148,7 +4173,7 @@ job_start(typval_T *argvars)
        /* Pass argv[] to mch_call_shell(). */
        argv = (char **)alloc(sizeof(char *) * (l->lv_len + 1));
        if (argv == NULL)
-           return job;
+           goto theend;
 #endif
        for (li = l->lv_first; li != NULL; li = li->li_next)
        {
@@ -4222,6 +4247,7 @@ theend:
 #else
     vim_free(ga.ga_data);
 #endif
+    free_job_options(&opt);
     return job;
 }
 

diff --git a/src/eval.c b/src/eval.c
index e7fbeaa55887a0d35d83249eb6757df1e4e5e1ff..b4e71668701ea6513e12c262d5bb2f40abc3627c 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -10321,9 +10321,9 @@ f_ch_setoptions(typval_T *argvars, typval_T *rettv UNUSED)
        return;
     clear_job_options(&opt);
     if (get_job_options(&argvars[1], &opt,
-               JO_CB_ALL + JO_TIMEOUT_ALL + JO_MODE_ALL) == FAIL)
-       return;
-    channel_set_options(channel, &opt);
+                             JO_CB_ALL + JO_TIMEOUT_ALL + JO_MODE_ALL) == OK)
+       channel_set_options(channel, &opt);
+    free_job_options(&opt);
 }
 
 /*
@@ -14889,9 +14889,9 @@ f_job_setoptions(typval_T *argvars, typval_T *rettv UNUSED)
     if (job == NULL)
        return;
     clear_job_options(&opt);
-    if (get_job_options(&argvars[1], &opt, JO_STOPONEXIT + JO_EXIT_CB) == FAIL)
-       return;
-    job_set_options(job, &opt);
+    if (get_job_options(&argvars[1], &opt, JO_STOPONEXIT + JO_EXIT_CB) == OK)
+       job_set_options(job, &opt);
+    free_job_options(&opt);
 }
 
 /*

diff --git a/src/proto/channel.pro b/src/proto/channel.pro
index b796d82bfec9b9bb5ceea4914fdd1592c4aa616b..e4ef00309ce01d97b911f7569847d3acef0f14e6 100644 (file)
--- a/src/proto/channel.pro
+++ b/src/proto/channel.pro
@@ -46,6 +46,7 @@ int channel_part_read(channel_T *channel);
 ch_mode_T channel_get_mode(channel_T *channel, int part);
 int channel_get_timeout(channel_T *channel, int part);
 void clear_job_options(jobopt_T *opt);
+void free_job_options(jobopt_T *opt);
 int get_job_options(typval_T *tv, jobopt_T *opt, int supported);
 channel_T *get_channel_arg(typval_T *tv, int check_open);
 void job_unref(job_T *job);

diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 119f71a35521f40530b3d243c7dd631933c95218..e1345279fd7f46e0fb2b16bd94d1a0d04f4630e3 100644 (file)
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -1231,5 +1231,17 @@ func Test_job_start_invalid()
   call assert_fails('call job_start("")', 'E474:')
 endfunc
 
+" This leaking memory.
+func Test_partial_in_channel_cycle()
+  let d = {}
+  let d.a = function('string', [d])
+  try
+    let d.b = ch_open('nowhere:123', {'close_cb': d.a})
+  catch
+    call assert_exception('E901:')
+  endtry
+  unlet d
+endfunc
+
 " Uncomment this to see what happens, output is in src/testdir/channellog.
 " call ch_logfile('channellog', 'w')

diff --git a/src/version.c b/src/version.c
index 7106a39eedb0cfdc7e177ab47f9f16f5516995c6..e0c0888e1a4364ea2e4d427d6530cceaeb14be04 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -748,6 +748,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1717,
 /**/
     1716,
 /**/