To: vim_dev@googlegroups.com Subject: Patch 7.4.1447 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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. Files: src/eval.c, src/channel.c *** ../vim-7.4.1446/src/eval.c 2016-02-27 21:27:16.550519087 +0100 --- src/eval.c 2016-02-28 15:28:31.019487430 +0100 *************** *** 7741,7747 **** /* * 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. */ int channel_unref(channel_T *channel) --- 7741,7747 ---- /* * 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 is no longer referenced. */ int channel_unref(channel_T *channel) *************** *** 7762,7767 **** --- 7762,7768 ---- 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,7802 **** --- 7797,7813 ---- * "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,10473 **** --- 10478,10487 ---- 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,15297 **** --- 15306,15314 ---- 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 *** ../vim-7.4.1446/src/channel.c 2016-02-27 21:52:56.622336150 +0100 --- src/channel.c 2016-02-28 15:34:49.323512394 +0100 *************** *** 311,318 **** --- 311,321 ---- } /* + * 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,352 **** --- 350,356 ---- { 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,780 **** --- 779,788 ---- } #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,1427 **** --- 1429,1438 ---- 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,1481 **** { if (item->cq_seq_nr == seq_nr) { ! ch_log(channel, "Invoking one-time callback"); /* Remove the item from the list first, if the callback * invokes ch_close() the list will be cleared. */ remove_cb_node(head, item); --- 1486,1493 ---- { if (item->cq_seq_nr == seq_nr) { ! 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,1494 **** item = item->cq_next; } if (!done) ! ch_log(channel, "Dropping message without callback"); } else if (callback != NULL || buffer != NULL) { --- 1500,1506 ---- item = item->cq_next; } if (!done) ! ch_logn(channel, "Dropping message %d without callback", seq_nr); } else if (callback != NULL || buffer != NULL) { *************** *** 1539,1544 **** --- 1551,1558 ---- invoke_callback(channel, callback, argv); } } + else if (msg != NULL) + ch_logs(channel, "Dropping message '%s'", (char *)msg); else ch_log(channel, "Dropping message"); *************** *** 1637,1642 **** --- 1651,1658 ---- /* 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,1709 **** --- 1720,1726 ---- 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,1726 **** --- 1738,1744 ---- { channel_T *channel; + ch_log(NULL, "channel_free_all()"); for (channel = first_channel; channel != NULL; channel = channel->ch_next) channel_clear(channel); } *************** *** 1798,1804 **** return OK; #endif } - ch_log(channel, "Nothing to read"); return FAIL; } --- 1816,1821 ---- *************** *** 1970,1980 **** */ int channel_read_json_block( ! channel_T *channel, ! int part, ! int timeout, ! int id, ! typval_T **rettv) { int more; sock_T fd; --- 1987,1997 ---- */ int channel_read_json_block( ! channel_T *channel, ! int part, ! int timeout, ! int id, ! typval_T **rettv) { int more; sock_T fd; *** ../vim-7.4.1446/src/version.c 2016-02-28 15:21:10.004115041 +0100 --- src/version.c 2016-02-28 15:23:19.446764741 +0100 *************** *** 745,746 **** --- 745,748 ---- { /* Add new patch number below this line */ + /**/ + 1447, /**/ -- TERRY GILLIAM PLAYED: PATSY (ARTHUR'S TRUSTY STEED), THE GREEN KNIGHT SOOTHSAYER, BRIDGEKEEPER, SIR GAWAIN (THE FIRST TO BE KILLED BY THE RABBIT) "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///