To: vim_dev@googlegroups.com Subject: Patch 7.4.1727 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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. Files: src/channel.c, src/eval.c, src/getchar.c, src/main.c, src/vim.h, src/proto/eval.pro, src/testdir/runtest.vim, src/testdir/test_channel.vim, runtime/doc/eval.txt *** ../vim-7.4.1726/src/channel.c 2016-04-08 17:07:09.542160709 +0200 --- src/channel.c 2016-04-14 12:37:05.582716400 +0200 *************** *** 458,465 **** 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. */ channel_free_channel(ch); } } --- 458,464 ---- ch_next = ch->ch_next; if ((ch->ch_copyID & mask) != (copyID & mask)) { ! /* Free the channel struct itself. */ channel_free_channel(ch); } } *************** *** 4006,4011 **** --- 4005,4021 ---- } } + /* + * 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,4020 **** { /* 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)) { job_free(job); } --- 4023,4029 ---- { /* Do not free the job when it has not ended yet and there is a * "stoponexit" flag or an exit callback. */ ! if (!job_still_useful(job)) { job_free(job); } *************** *** 4036,4042 **** job_T *job; for (job = first_job; job != NULL; job = job->jv_next) ! if ((job->jv_copyID & mask) != (copyID & mask)) { /* Free the channel and ordinary items it contains, but don't * recurse into Lists, Dictionaries etc. */ --- 4045,4052 ---- job_T *job; for (job = first_job; job != NULL; job = job->jv_next) ! 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,4064 **** for (job = first_job; job != NULL; job = job_next) { job_next = job->jv_next; ! if ((job->jv_copyID & mask) != (copyID & mask)) { ! /* Free the channel and ordinary items it contains, but don't ! * recurse into Lists, Dictionaries etc. */ job_free_job(job); } } --- 4065,4074 ---- for (job = first_job; job != NULL; job = job_next) { job_next = job->jv_next; ! if ((job->jv_copyID & mask) != (copyID & mask) ! && !job_still_useful(job)) { ! /* Free the job struct itself. */ job_free_job(job); } } *** ../vim-7.4.1726/src/eval.c 2016-04-12 22:18:47.670129251 +0200 --- src/eval.c 2016-04-14 12:43:01.882944041 +0200 *************** *** 373,378 **** --- 373,379 ---- {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,585 **** --- 581,587 ---- 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,1035 **** ga_clear(&ga_scripts); /* unreferenced lists and dicts */ ! (void)garbage_collect(); /* functions */ free_all_functions(); --- 1031,1037 ---- ga_clear(&ga_scripts); /* unreferenced lists and dicts */ ! (void)garbage_collect(FALSE); /* functions */ free_all_functions(); *************** *** 6889,6894 **** --- 6891,6899 ---- return current_copyID; } + /* Used by get_func_tv() */ + static garray_T funcargs = GA_EMPTY; + /* * Garbage collection for lists and dictionaries. * *************** *** 6911,6920 **** /* * Do garbage collection for lists and dicts. * Return TRUE if some memory was freed. */ int ! garbage_collect(void) { int copyID; int abort = FALSE; --- 6916,6926 ---- /* * 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(int testing) { int copyID; int abort = FALSE; *************** *** 6928,6937 **** tabpage_T *tp; #endif ! /* 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. */ --- 6934,6946 ---- tabpage_T *tp; #endif ! 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,6994 **** --- 6998,7008 ---- 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,7040 **** if (did_free_funccal) /* When a funccal was freed some more items might be garbage * collected, so run again. */ ! (void)garbage_collect(); } else if (p_verbose > 0) { --- 7048,7054 ---- if (did_free_funccal) /* When a funccal was freed some more items might be garbage * collected, so run again. */ ! (void)garbage_collect(testing); } else if (p_verbose > 0) { *************** *** 8424,8429 **** --- 8438,8444 ---- {"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,8903 **** --- 8911,8936 ---- 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,12323 **** --- 12351,12367 ---- } /* + * "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 *** ../vim-7.4.1726/src/getchar.c 2016-02-23 14:52:31.881232212 +0100 --- src/getchar.c 2016-04-14 11:00:48.555441735 +0200 *************** *** 1523,1529 **** updatescript(0); #ifdef FEAT_EVAL if (may_garbage_collect) ! garbage_collect(); #endif } --- 1523,1529 ---- updatescript(0); #ifdef FEAT_EVAL if (may_garbage_collect) ! garbage_collect(FALSE); #endif } *************** *** 1571,1577 **** /* Do garbage collection when garbagecollect() was called previously and * we are now at the toplevel. */ if (may_garbage_collect && want_garbage_collect) ! garbage_collect(); #endif /* --- 1571,1577 ---- /* Do garbage collection when garbagecollect() was called previously and * we are now at the toplevel. */ if (may_garbage_collect && want_garbage_collect) ! garbage_collect(FALSE); #endif /* *** ../vim-7.4.1726/src/main.c 2016-04-06 23:06:18.586052503 +0200 --- src/main.c 2016-04-14 11:00:58.435338807 +0200 *************** *** 1531,1537 **** #endif #ifdef FEAT_EVAL if (garbage_collect_at_exit) ! garbage_collect(); #endif #if defined(WIN32) && defined(FEAT_MBYTE) free_cmd_argsW(); --- 1531,1537 ---- #endif #ifdef FEAT_EVAL if (garbage_collect_at_exit) ! garbage_collect(FALSE); #endif #if defined(WIN32) && defined(FEAT_MBYTE) free_cmd_argsW(); *** ../vim-7.4.1726/src/vim.h 2016-03-28 19:16:15.669846492 +0200 --- src/vim.h 2016-04-14 11:18:58.496104039 +0200 *************** *** 1868,1874 **** #define VV_NULL 65 #define VV_NONE 66 #define VV_VIM_DID_ENTER 67 ! #define VV_LEN 68 /* number of v: vars */ /* used for v_number in VAR_SPECIAL */ #define VVAL_FALSE 0L --- 1868,1875 ---- #define VV_NULL 65 #define VV_NONE 66 #define VV_VIM_DID_ENTER 67 ! #define VV_TESTING 68 ! #define VV_LEN 69 /* number of v: vars */ /* used for v_number in VAR_SPECIAL */ #define VVAL_FALSE 0L *** ../vim-7.4.1726/src/proto/eval.pro 2016-04-08 17:07:09.546160667 +0200 --- src/proto/eval.pro 2016-04-14 11:01:12.883188293 +0200 *************** *** 49,55 **** 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); --- 49,54 ---- *************** *** 66,79 **** 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 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); --- 65,77 ---- 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(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(dict_T *d); dictitem_T *dictitem_alloc(char_u *key); void dictitem_free(dictitem_T *item); *** ../vim-7.4.1726/src/testdir/runtest.vim 2016-03-30 20:50:41.905696041 +0200 --- src/testdir/runtest.vim 2016-04-14 11:17:30.929013730 +0200 *************** *** 60,65 **** --- 60,68 ---- 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' *** ../vim-7.4.1726/src/testdir/test_channel.vim 2016-04-07 21:40:34.066966030 +0200 --- src/testdir/test_channel.vim 2016-04-14 11:48:31.497576059 +0200 *************** *** 183,189 **** call assert_equal('got it', s:responseMsg) " Collect garbage, tests that our handle isn't collected. ! call garbagecollect() " check setting options (without testing the effect) call ch_setoptions(handle, {'callback': 's:NotUsed'}) --- 183,189 ---- call assert_equal('got it', s:responseMsg) " Collect garbage, tests that our handle isn't collected. ! call garbagecollect_for_testing() " check setting options (without testing the effect) call ch_setoptions(handle, {'callback': 's:NotUsed'}) *************** *** 1231,1237 **** 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]) --- 1231,1237 ---- call assert_fails('call job_start("")', 'E474:') endfunc ! " This was leaking memory. func Test_partial_in_channel_cycle() let d = {} let d.a = function('string', [d]) *************** *** 1243,1247 **** --- 1243,1255 ---- 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') *** ../vim-7.4.1726/runtime/doc/eval.txt 2016-04-03 20:57:17.009726516 +0200 --- runtime/doc/eval.txt 2016-04-14 11:31:50.120078822 +0200 *************** *** 1705,1710 **** --- 1723,1731 ---- 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 *************** *** 1887,1895 **** 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}]) Funcref reference to function {name} garbagecollect( [{atexit}]) none free memory, breaking cyclic references 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}]) --- 1908,1917 ---- 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}]) 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}]) *************** *** 3635,3653 **** 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. 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. get({list}, {idx} [, {default}]) *get()* Get item {idx} from |List| {list}. When this item is not available return {default}. Return zero when {default} is --- 3678,3704 ---- garbagecollect([{atexit}]) *garbagecollect()* ! 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 *** ../vim-7.4.1726/src/version.c 2016-04-13 21:14:31.256124229 +0200 --- src/version.c 2016-04-14 11:17:53.612778061 +0200 *************** *** 750,751 **** --- 750,753 ---- { /* Add new patch number below this line */ + /**/ + 1727, /**/ -- GOD: That is your purpose Arthur ... the Quest for the Holy Grail ... "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 ///