# HG changeset patch # User Christian Brabandt # Date 1460058305 -7200 # Node ID 03250bc0c63aad58f86f10e3a423ae62fa9defe9 # Parent 593b17f02b78c9a46a3be835633bc1517a2aa51e commit https://github.com/vim/vim/commit/0e4c1de5560c7f8b4cae539ec8cff0949daba3fc Author: Bram Moolenaar Date: Thu Apr 7 21:40:38 2016 +0200 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 --- 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, t 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, t } } } + +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 free_tv(listtv); } } + free_job_options(&opt); } /* @@ -3175,6 +3181,7 @@ ch_raw_common(typval_T *argvars, typval_ 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,6 +3552,9 @@ handle_io(typval_T *item, int part, jobo return OK; } +/* + * Clear a jobopt_T before using it. + */ void clear_job_options(jobopt_T *opt) { @@ -3552,6 +3562,22 @@ clear_job_options(jobopt_T *opt) } /* + * 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. */ static int @@ -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 --- a/src/eval.c +++ b/src/eval.c @@ -10321,9 +10321,9 @@ f_ch_setoptions(typval_T *argvars, typva 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, typv 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 --- 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 --- 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 --- a/src/version.c +++ b/src/version.c @@ -749,6 +749,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 1717, +/**/ 1716, /**/ 1715,