# HG changeset patch # User Christian Brabandt # Date 1456671605 -3600 # Node ID aec8f8ce8e4cb9c2bbbf5942afc1dd28a1692b8a # Parent 9df48ab0f290547253f34fb960c9e673ebddbf3f commit https://github.com/vim/vim/commit/d6051b5eb83687f60bb4a2f3d5cd23fe8b290eb4 Author: Bram Moolenaar Date: Sun Feb 28 15:49:03 2016 +0100 patch 7.4.1447 Problem: Memory leak when using ch_read(). (Dominique Pelle) No log message when stopping a job and a few other situations. Too many "Nothing to read" messages. Channels are not freed. Solution: Free the listtv. Add more log messages. Remove "Nothing to read" message. Remove the channel from the job when its refcount becomes zero. diff --git a/src/channel.c b/src/channel.c --- a/src/channel.c +++ b/src/channel.c @@ -311,8 +311,11 @@ add_channel(void) } /* + * Called when the refcount of a channel is zero. * Return TRUE if "channel" has a callback and the associated job wasn't * killed. + * If the job was killed the channel is not expected to work anymore. + * If there is no callback then nobody can get readahead. */ static int channel_still_useful(channel_T *channel) @@ -347,6 +350,7 @@ channel_free(channel_T *channel) { channel_close(channel, TRUE); channel_clear(channel); + ch_log(channel, "Freeing channel"); if (channel->ch_next != NULL) channel->ch_next->ch_prev = channel->ch_prev; if (channel->ch_prev == NULL) @@ -775,6 +779,10 @@ channel_set_pipes(channel_T *channel, so } #endif +/* + * Sets the job the channel is associated with. + * This does not keep a refcount, when the job is freed ch_job is cleared. + */ void channel_set_job(channel_T *channel, job_T *job) { @@ -1421,7 +1429,10 @@ may_invoke_callback(channel_T *channel, if (callback == NULL && buffer == NULL) { while ((msg = channel_get(channel, part)) != NULL) + { + ch_logs(channel, "Dropping message '%s'", (char *)msg); vim_free(msg); + } return FALSE; } @@ -1475,7 +1486,8 @@ may_invoke_callback(channel_T *channel, { if (item->cq_seq_nr == seq_nr) { - ch_log(channel, "Invoking one-time callback"); + ch_logs(channel, "Invoking one-time callback '%s'", + (char *)item->cq_callback); /* Remove the item from the list first, if the callback * invokes ch_close() the list will be cleared. */ remove_cb_node(head, item); @@ -1488,7 +1500,7 @@ may_invoke_callback(channel_T *channel, item = item->cq_next; } if (!done) - ch_log(channel, "Dropping message without callback"); + ch_logn(channel, "Dropping message %d without callback", seq_nr); } else if (callback != NULL || buffer != NULL) { @@ -1539,6 +1551,8 @@ may_invoke_callback(channel_T *channel, invoke_callback(channel, callback, argv); } } + else if (msg != NULL) + ch_logs(channel, "Dropping message '%s'", (char *)msg); else ch_log(channel, "Dropping message"); @@ -1637,6 +1651,8 @@ channel_close(channel_T *channel, int in /* 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; ++channel->ch_refcount; @@ -1704,6 +1720,7 @@ channel_clear_one(channel_T *channel, in void channel_clear(channel_T *channel) { + ch_log(channel, "Clearing channel"); channel_clear_one(channel, PART_SOCK); #ifdef CHANNEL_PIPES channel_clear_one(channel, PART_OUT); @@ -1721,6 +1738,7 @@ channel_free_all(void) { channel_T *channel; + ch_log(NULL, "channel_free_all()"); for (channel = first_channel; channel != NULL; channel = channel->ch_next) channel_clear(channel); } @@ -1798,7 +1816,6 @@ channel_wait(channel_T *channel, sock_T return OK; #endif } - ch_log(channel, "Nothing to read"); return FAIL; } @@ -1970,11 +1987,11 @@ channel_read_block(channel_T *channel, i */ int channel_read_json_block( - channel_T *channel, - int part, - int timeout, - int id, - typval_T **rettv) + channel_T *channel, + int part, + int timeout, + int id, + typval_T **rettv) { int more; sock_T fd; diff --git a/src/eval.c b/src/eval.c --- a/src/eval.c +++ b/src/eval.c @@ -7741,7 +7741,7 @@ failret: /* * Decrement the reference count on "channel" and maybe free it when it goes * down to zero. Don't free it if there is a pending action. - * Returns TRUE when the channel was freed. + * Returns TRUE when the channel is no longer referenced. */ int channel_unref(channel_T *channel) @@ -7762,6 +7762,7 @@ static job_T *first_job = NULL; job_free(job_T *job) { # ifdef FEAT_CHANNEL + ch_log(job->jv_channel, "Freeing job"); if (job->jv_channel != NULL) { /* The link from the channel to the job doesn't count as a reference, @@ -7796,7 +7797,17 @@ job_unref(job_T *job) * "stoponexit" flag or an exit callback. */ if (job->jv_status != JOB_STARTED || (job->jv_stoponexit == NULL && job->jv_exit_cb == NULL)) + { job_free(job); + } + else if (job->jv_channel != NULL) + { + /* Do remove the link to the channel, otherwise it hangs + * around until Vim exits. See job_free() for refcount. */ + job->jv_channel->ch_job = NULL; + channel_unref(job->jv_channel); + job->jv_channel = NULL; + } } } @@ -10467,7 +10478,10 @@ common_channel_read(typval_T *argvars, t id = opt.jo_id; channel_read_json_block(channel, part, timeout, id, &listtv); if (listtv != NULL) + { *rettv = *listtv; + vim_free(listtv); + } else { rettv->v_type = VAR_SPECIAL; @@ -15292,6 +15306,9 @@ f_job_stop(typval_T *argvars UNUSED, typ return; } } +# ifdef FEAT_CHANNEL + ch_logs(job->jv_channel, "Stopping job with '%s'", (char *)arg); +# endif if (mch_stop_job(job, arg) == FAIL) rettv->vval.v_number = 0; else diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -744,6 +744,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 1447, +/**/ 1446, /**/ 1445,