]> granicus.if.org Git - vim/commitdiff
patch 7.4.1727 v7.4.1727
authorBram Moolenaar <Bram@vim.org>
Thu, 14 Apr 2016 10:46:51 +0000 (12:46 +0200)
committerBram Moolenaar <Bram@vim.org>
Thu, 14 Apr 2016 10:46:51 +0000 (12:46 +0200)
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.

runtime/doc/eval.txt
src/channel.c
src/eval.c
src/getchar.c
src/main.c
src/proto/eval.pro
src/testdir/runtest.vim
src/testdir/test_channel.vim
src/version.c
src/vim.h

index 1b4b3b7b978d04db7f13bf79a3f7857aa607a65e..93bf3f1ac4ace934f9fecd2213ca60dc6da53afb 100644 (file)
@@ -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 |Funcref|.
 
 Special                |v:false|, |v:true|, |v:none| and |v:null|.  *Special*
 
-Job            Used for a job, see |job_start()|. *Job*
+Job            Used for a job, see |job_start()|. *Job* *Jobs*
 
-Channel                Used for a channel, see |ch_open()|. *Channel*
+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 returned by the terminal for the |t_RV|
                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 {lnum}
 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
index 83d057dbb089fd84859a35621edbda8e2e67c624..72484ecb0483d7c1c4ca0f98a94625163c4ea8c6 100644 (file)
@@ -458,8 +458,7 @@ free_unused_channels(int copyID, int mask)
        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, int mask)
     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);
        }
     }
index a14a721a0e0276bb7fbd57b26db92244dd0bf74d..86580450fca5fe5f4f1e3b82f86d67686a5d80a2 100644 (file)
@@ -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 *argvars, typval_T *rettv);
 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)
@@ -12317,6 +12350,17 @@ f_garbagecollect(typval_T *argvars, typval_T *rettv UNUSED)
        garbage_collect_at_exit = TRUE;
 }
 
+/*
+ * "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
  */
index 4a225e1b640405c7be5ab7efa938496aec5f1e82..c771117add2cac02bdc587d7bb0eb9c8230c3dff 100644 (file)
@@ -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
 
     /*
index dc7b702bd9c97f86f6461e61cf3f20d278a9a2c3..ee375000327c376a39352b799ade95655da11028 100644 (file)
@@ -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();
index 38392b9bd2d48fa6c0ab2a8a25137588512007da..9fda13c9c5faccf9b80c6c939a84854ec6a81540 100644 (file)
@@ -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 *tv, listitem_T *item);
 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);
index 4d75af3ee9b3f43b1478f832c91914b1203df30a..2b38981bb134d170bf9a5e0c77b8f2bdf37eea2f 100644 (file)
@@ -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'
index e1345279fd7f46e0fb2b16bd94d1a0d04f4630e3..05df50af433598f29e7972b7e659fef54446a172 100644 (file)
@@ -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')
index cf7d48cb68a79dec38ef984f4c5fa51bb9caa3ca..6c94bc64b23f778f1b0fb3917c002dca7b9bdd78 100644 (file)
@@ -748,6 +748,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1727,
 /**/
     1726,
 /**/
index 36cc19a27a06d7b498fc08554dd062d5c02714d5..9e2ab2161a53bdcf742bd443f6e9ef45e1ef64d7 100644 (file)
--- 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