# HG changeset patch # User Christian Brabandt # Date 1460631606 -7200 # Node ID ed0b39dd7fd6ed43be15940727f29cfcf4c049f6 # Parent 9f57791bc92260e470494393405b4b680482b4d4 commit https://github.com/vim/vim/commit/ebf7dfa6f121c82f97d2adca3d45fbaba9ad8f7e Author: Bram Moolenaar Date: Thu Apr 14 12:46:51 2016 +0200 patch 7.4.1727 Problem: Cannot detect a crash in tests when caused by garbagecollect(). Solution: Add garbagecollect_for_testing(). Do not free a job if is still useful. diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt --- a/runtime/doc/eval.txt +++ b/runtime/doc/eval.txt @@ -1,4 +1,4 @@ -*eval.txt* For Vim version 7.4. Last change: 2016 Apr 12 +*eval.txt* For Vim version 7.4. Last change: 2016 Apr 14 VIM REFERENCE MANUAL by Bram Moolenaar @@ -61,9 +61,9 @@ Funcref A reference to a function |Func Special |v:false|, |v:true|, |v:none| and |v:null|. *Special* -Job Used for a job, see |job_start()|. *Job* - -Channel Used for a channel, see |ch_open()|. *Channel* +Job Used for a job, see |job_start()|. *Job* *Jobs* + +Channel Used for a channel, see |ch_open()|. *Channel* *Channels* The Number and String types are converted automatically, depending on how they are used. @@ -1723,6 +1723,9 @@ v:termresponse The escape sequence retur always 95 or bigger). Pc is always zero. {only when compiled with |+termresponse| feature} + *v:testing* *testing-variable* +v:testing Must be set before using `garbagecollect_for_testing()`. + *v:this_session* *this_session-variable* v:this_session Full filename of the last loaded or saved session file. See |:mksession|. It is allowed to set this variable. When no @@ -1905,9 +1908,10 @@ foldlevel( {lnum}) Number fold level at foldtext() String line displayed for closed fold foldtextresult( {lnum}) String text for closed fold at {lnum} foreground() Number bring the Vim window to the foreground -function({name} [, {arglist}] [, {dict}]) +function( {name} [, {arglist}] [, {dict}]) Funcref reference to function {name} garbagecollect( [{atexit}]) none free memory, breaking cyclic references +garbagecollect_for_testing() none free memory right now get( {list}, {idx} [, {def}]) any get item {idx} from {list} or {def} get( {dict}, {key} [, {def}]) any get item {key} from {dict} or {def} getbufline( {expr}, {lnum} [, {end}]) @@ -3674,19 +3678,27 @@ function({name} [, {arglist}] [, {dict}] garbagecollect([{atexit}]) *garbagecollect()* - Cleanup unused |Lists| and |Dictionaries| that have circular - references. There is hardly ever a need to invoke this - function, as it is automatically done when Vim runs out of - memory or is waiting for the user to press a key after - 'updatetime'. Items without circular references are always - freed when they become unused. + Cleanup unused |Lists|, |Dictionaries|, |Channels| and |Jobs| + that have circular references. + + There is hardly ever a need to invoke this function, as it is + automatically done when Vim runs out of memory or is waiting + for the user to press a key after 'updatetime'. Items without + circular references are always freed when they become unused. This is useful if you have deleted a very big |List| and/or |Dictionary| with circular references in a script that runs for a long time. + When the optional {atexit} argument is one, garbage collection will also be done when exiting Vim, if it wasn't done before. This is useful when checking for memory leaks. +garbagecollect_for_testing() *garbagecollect_for_testing()* + Like garbagecollect(), but executed right away. This must + only be called directly to avoid any structure to exist + internally, and |v:testing| must have been set before calling + any function. + get({list}, {idx} [, {default}]) *get()* Get item {idx} from |List| {list}. When this item is not available return {default}. Return zero when {default} is diff --git a/src/channel.c b/src/channel.c --- a/src/channel.c +++ b/src/channel.c @@ -458,8 +458,7 @@ free_unused_channels(int copyID, int mas ch_next = ch->ch_next; if ((ch->ch_copyID & mask) != (copyID & mask)) { - /* Free the channel and ordinary items it contains, but don't - * recurse into Lists, Dictionaries etc. */ + /* Free the channel struct itself. */ channel_free_channel(ch); } } @@ -4006,6 +4005,17 @@ job_free(job_T *job) } } +/* + * Return TRUE if the job should not be freed yet. Do not free the job when + * it has not ended yet and there is a "stoponexit" flag or an exit callback. + */ + static int +job_still_useful(job_T *job) +{ + return job->jv_status == JOB_STARTED + && (job->jv_stoponexit != NULL || job->jv_exit_cb != NULL); +} + void job_unref(job_T *job) { @@ -4013,8 +4023,7 @@ job_unref(job_T *job) { /* Do not free the job when it has not ended yet and there is a * "stoponexit" flag or an exit callback. */ - if (job->jv_status != JOB_STARTED - || (job->jv_stoponexit == NULL && job->jv_exit_cb == NULL)) + if (!job_still_useful(job)) { job_free(job); } @@ -4036,7 +4045,8 @@ free_unused_jobs_contents(int copyID, in job_T *job; for (job = first_job; job != NULL; job = job->jv_next) - if ((job->jv_copyID & mask) != (copyID & mask)) + if ((job->jv_copyID & mask) != (copyID & mask) + && !job_still_useful(job)) { /* Free the channel and ordinary items it contains, but don't * recurse into Lists, Dictionaries etc. */ @@ -4055,10 +4065,10 @@ free_unused_jobs(int copyID, int mask) for (job = first_job; job != NULL; job = job_next) { job_next = job->jv_next; - if ((job->jv_copyID & mask) != (copyID & mask)) + if ((job->jv_copyID & mask) != (copyID & mask) + && !job_still_useful(job)) { - /* Free the channel and ordinary items it contains, but don't - * recurse into Lists, Dictionaries etc. */ + /* Free the job struct itself. */ job_free_job(job); } } diff --git a/src/eval.c b/src/eval.c --- a/src/eval.c +++ b/src/eval.c @@ -373,6 +373,7 @@ static struct vimvar {VV_NAME("null", VAR_SPECIAL), VV_RO}, {VV_NAME("none", VAR_SPECIAL), VV_RO}, {VV_NAME("vim_did_enter", VAR_NUMBER), VV_RO}, + {VV_NAME("testing", VAR_NUMBER), 0}, }; /* shorthand */ @@ -580,6 +581,7 @@ static void f_foldtextresult(typval_T *a static void f_foreground(typval_T *argvars, typval_T *rettv); static void f_function(typval_T *argvars, typval_T *rettv); static void f_garbagecollect(typval_T *argvars, typval_T *rettv); +static void f_garbagecollect_for_testing(typval_T *argvars, typval_T *rettv); static void f_get(typval_T *argvars, typval_T *rettv); static void f_getbufline(typval_T *argvars, typval_T *rettv); static void f_getbufvar(typval_T *argvars, typval_T *rettv); @@ -1029,7 +1031,7 @@ eval_clear(void) ga_clear(&ga_scripts); /* unreferenced lists and dicts */ - (void)garbage_collect(); + (void)garbage_collect(FALSE); /* functions */ free_all_functions(); @@ -6889,6 +6891,9 @@ get_copyID(void) return current_copyID; } +/* Used by get_func_tv() */ +static garray_T funcargs = GA_EMPTY; + /* * Garbage collection for lists and dictionaries. * @@ -6911,10 +6916,11 @@ get_copyID(void) /* * Do garbage collection for lists and dicts. + * When "testing" is TRUE this is called from garbagecollect_for_testing(). * Return TRUE if some memory was freed. */ int -garbage_collect(void) +garbage_collect(int testing) { int copyID; int abort = FALSE; @@ -6928,10 +6934,13 @@ garbage_collect(void) tabpage_T *tp; #endif - /* Only do this once. */ - want_garbage_collect = FALSE; - may_garbage_collect = FALSE; - garbage_collect_at_exit = FALSE; + if (!testing) + { + /* Only do this once. */ + want_garbage_collect = FALSE; + may_garbage_collect = FALSE; + garbage_collect_at_exit = FALSE; + } /* We advance by two because we add one for items referenced through * previous_funccal. */ @@ -6989,6 +6998,11 @@ garbage_collect(void) abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL); } + /* function call arguments, if v:testing is set. */ + for (i = 0; i < funcargs.ga_len; ++i) + abort = abort || set_ref_in_item(((typval_T **)funcargs.ga_data)[i], + copyID, NULL, NULL); + /* v: vars */ abort = abort || set_ref_in_ht(&vimvarht, copyID, NULL); @@ -7034,7 +7048,7 @@ garbage_collect(void) if (did_free_funccal) /* When a funccal was freed some more items might be garbage * collected, so run again. */ - (void)garbage_collect(); + (void)garbage_collect(testing); } else if (p_verbose > 0) { @@ -8424,6 +8438,7 @@ static struct fst {"foreground", 0, 0, f_foreground}, {"function", 1, 3, f_function}, {"garbagecollect", 0, 1, f_garbagecollect}, + {"garbagecollect_for_testing", 0, 0, f_garbagecollect_for_testing}, {"get", 2, 3, f_get}, {"getbufline", 2, 3, f_getbufline}, {"getbufvar", 2, 3, f_getbufvar}, @@ -8896,8 +8911,26 @@ get_func_tv( ret = FAIL; if (ret == OK) + { + int i = 0; + + if (get_vim_var_nr(VV_TESTING)) + { + /* Prepare for calling garbagecollect_for_testing(), need to know + * what variables are used on the call stack. */ + if (funcargs.ga_itemsize == 0) + ga_init2(&funcargs, (int)sizeof(typval_T *), 50); + for (i = 0; i < argcount; ++i) + if (ga_grow(&funcargs, 1) == OK) + ((typval_T **)funcargs.ga_data)[funcargs.ga_len++] = + &argvars[i]; + } + ret = call_func(name, len, rettv, argcount, argvars, firstline, lastline, doesrange, evaluate, partial, selfdict); + + funcargs.ga_len -= i; + } else if (!aborting()) { if (argcount == MAX_FUNC_ARGS) @@ -12318,6 +12351,17 @@ f_garbagecollect(typval_T *argvars, typv } /* + * "garbagecollect_for_testing()" function + */ + static void +f_garbagecollect_for_testing(typval_T *argvars UNUSED, typval_T *rettv UNUSED) +{ + /* This is dangerous, any Lists and Dicts used internally may be freed + * while still in use. */ + garbage_collect(TRUE); +} + +/* * "get()" function */ static void diff --git a/src/getchar.c b/src/getchar.c --- a/src/getchar.c +++ b/src/getchar.c @@ -1523,7 +1523,7 @@ before_blocking(void) updatescript(0); #ifdef FEAT_EVAL if (may_garbage_collect) - garbage_collect(); + garbage_collect(FALSE); #endif } @@ -1571,7 +1571,7 @@ vgetc(void) /* Do garbage collection when garbagecollect() was called previously and * we are now at the toplevel. */ if (may_garbage_collect && want_garbage_collect) - garbage_collect(); + garbage_collect(FALSE); #endif /* diff --git a/src/main.c b/src/main.c --- a/src/main.c +++ b/src/main.c @@ -1531,7 +1531,7 @@ getout(int exitval) #endif #ifdef FEAT_EVAL if (garbage_collect_at_exit) - garbage_collect(); + garbage_collect(FALSE); #endif #if defined(WIN32) && defined(FEAT_MBYTE) free_cmd_argsW(); diff --git a/src/proto/eval.pro b/src/proto/eval.pro --- a/src/proto/eval.pro +++ b/src/proto/eval.pro @@ -49,7 +49,6 @@ void partial_unref(partial_T *pt); list_T *list_alloc(void); int rettv_list_alloc(typval_T *rettv); void list_unref(list_T *l); -void list_free_internal(list_T *l); void list_free(list_T *l); listitem_T *listitem_alloc(void); void listitem_free(listitem_T *item); @@ -66,14 +65,13 @@ int list_insert_tv(list_T *l, typval_T * void list_insert(list_T *l, listitem_T *ni, listitem_T *item); void vimlist_remove(list_T *l, listitem_T *item, listitem_T *item2); int get_copyID(void); -int garbage_collect(void); +int garbage_collect(int testing); int set_ref_in_ht(hashtab_T *ht, int copyID, list_stack_T **list_stack); int set_ref_in_list(list_T *l, int copyID, ht_stack_T **ht_stack); int set_ref_in_item(typval_T *tv, int copyID, ht_stack_T **ht_stack, list_stack_T **list_stack); dict_T *dict_alloc(void); int rettv_dict_alloc(typval_T *rettv); void dict_unref(dict_T *d); -void dict_free_internal(dict_T *d); void dict_free(dict_T *d); dictitem_T *dictitem_alloc(char_u *key); void dictitem_free(dictitem_T *item); diff --git a/src/testdir/runtest.vim b/src/testdir/runtest.vim --- a/src/testdir/runtest.vim +++ b/src/testdir/runtest.vim @@ -60,6 +60,9 @@ let $HOME = '/does/not/exist' let s:srcdir = expand('%:p:h:h') +" Prepare for calling garbagecollect_for_testing(). +let v:testing = 1 + " Support function: get the alloc ID by name. function GetAllocId(name) exe 'split ' . s:srcdir . '/alloc.h' 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 @@ -183,7 +183,7 @@ func s:communicate(port) call assert_equal('got it', s:responseMsg) " Collect garbage, tests that our handle isn't collected. - call garbagecollect() + call garbagecollect_for_testing() " check setting options (without testing the effect) call ch_setoptions(handle, {'callback': 's:NotUsed'}) @@ -1231,7 +1231,7 @@ func Test_job_start_invalid() call assert_fails('call job_start("")', 'E474:') endfunc -" This leaking memory. +" This was leaking memory. func Test_partial_in_channel_cycle() let d = {} let d.a = function('string', [d]) @@ -1243,5 +1243,13 @@ func Test_partial_in_channel_cycle() unlet d endfunc +func Test_using_freed_memory() + let g:a = job_start(['ls']) + sleep 10m + call garbagecollect_for_testing() +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 */ /**/ + 1727, +/**/ 1726, /**/ 1725, diff --git a/src/vim.h b/src/vim.h --- a/src/vim.h +++ b/src/vim.h @@ -1868,7 +1868,8 @@ typedef int sock_T; #define VV_NULL 65 #define VV_NONE 66 #define VV_VIM_DID_ENTER 67 -#define VV_LEN 68 /* number of v: vars */ +#define VV_TESTING 68 +#define VV_LEN 69 /* number of v: vars */ /* used for v_number in VAR_SPECIAL */ #define VVAL_FALSE 0L