patch 8.2.0684: Vim9: memory leak when using lambda

Problem:    Vim9: memory leak when using lambda.
Solution:   Move the funccal context to the partial. Free the function when
            exiting.

diff --git a/src/eval.c b/src/eval.c
index 31dde2faed6667aa2a2e44e4316d01a99a276737..72cb0897affcc0fc79b163a795da86e9ea3a8370 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -3703,6 +3703,24 @@ partial_free(partial_T *pt)
     }
     else
        func_ptr_unref(pt->pt_func);
+
+    if (pt->pt_funcstack != NULL)
+    {
+       // Decrease the reference count for the context of a closure.  If down
+       // to zero free it and clear the variables on the stack.
+       if (--pt->pt_funcstack->fs_refcount == 0)
+       {
+           garray_T    *gap = &pt->pt_funcstack->fs_ga;
+           typval_T    *stack = gap->ga_data;
+
+           for (i = 0; i < gap->ga_len; ++i)
+               clear_tv(stack + i);
+           ga_clear(gap);
+           vim_free(pt->pt_funcstack);
+       }
+       pt->pt_funcstack = NULL;
+    }
+
     vim_free(pt);
 }
 
@@ -4336,6 +4354,15 @@ set_ref_in_item(
            for (i = 0; i < pt->pt_argc; ++i)
                abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID,
                                                        ht_stack, list_stack);
+           if (pt->pt_funcstack != NULL)
+           {
+               typval_T    *stack = pt->pt_funcstack->fs_ga.ga_data;
+
+               for (i = 0; i < pt->pt_funcstack->fs_ga.ga_len; ++i)
+                   abort = abort || set_ref_in_item(stack + i, copyID,
+                                                        ht_stack, list_stack);
+           }
+
        }
     }
 #ifdef FEAT_JOB_CHANNEL

diff --git a/src/structs.h b/src/structs.h
index 601194e12f8a189ac56353d95f1d9432b3df9efa..7fd325a825879dd46e62b1f950d185fd57b9b71d 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1777,6 +1777,21 @@ typedef struct {
     typval_T   *basetv;        // base for base->method()
 } funcexe_T;
 
+/*
+ * Structure to hold the context of a compiled function, used by closures
+ * defined in that function.
+ */
+typedef struct funcstack_S
+{
+    garray_T   fs_ga;          // contains the stack, with:
+                               // - arguments
+                               // - frame
+                               // - local variables
+
+    int                fs_refcount;    // nr of closures referencing this funcstack
+    int                fs_copyID;      // for garray_T collection
+} funcstack_T;
+
 struct partial_S
 {
     int                pt_refcount;    // reference count
@@ -1786,8 +1801,16 @@ struct partial_S
                                // with pt_name
     int                pt_auto;        // when TRUE the partial was created for using
                                // dict.member in handle_subscript()
+
+    // For a compiled closure: the arguments and local variables.
+    garray_T   *pt_ectx_stack;     // where to find local vars
+    int                pt_ectx_frame;      // index of function frame in uf_ectx_stack
+    funcstack_T        *pt_funcstack;      // copy of stack, used after context
+                                   // function returns
+
     int                pt_argc;        // number of arguments
     typval_T   *pt_argv;       // arguments in allocated array
+
     dict_T     *pt_dict;       // dict for "self"
 };
 

diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index a01e27c72961f98fac524781646428607dd9aaed..fe47513ee9f55b5a966cf822a4b45c544d40b9d4 100644 (file)
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -680,13 +680,6 @@ def Test_closure_two_refs()
   unlet g:Read
 enddef
 
-" TODO: fix memory leak when using same function again.
-def MakeTwoRefs_2()
-  let local = ['some']
-  g:Extend = {s -> local->add(s)}
-  g:Read = {-> local}
-enddef
-
 def ReadRef(Ref: func(): list<string>): string
   return join(Ref(), ' ')
 enddef
@@ -696,7 +689,7 @@ def ExtendRef(Ref: func(string), add: string)
 enddef
 
 def Test_closure_two_indirect_refs()
-  MakeTwoRefs_2()
+  MakeTwoRefs()
   assert_equal('some', ReadRef(g:Read))
   ExtendRef(g:Extend, 'more')
   assert_equal('some more', ReadRef(g:Read))
@@ -707,4 +700,5 @@ def Test_closure_two_indirect_refs()
   unlet g:Read
 enddef
 
+
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

diff --git a/src/userfunc.c b/src/userfunc.c
index 6aee37795391eec8b8d552725a52bbb2621f74a0..4dac28126f99ba3721b747fb7be154df4e137232 100644 (file)
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -1016,16 +1016,17 @@ func_clear(ufunc_T *fp, int force)
 /*
  * Free a function and remove it from the list of functions.  Does not free
  * what a function contains, call func_clear() first.
+ * When "force" is TRUE we are exiting.
  */
     static void
-func_free(ufunc_T *fp)
+func_free(ufunc_T *fp, int force)
 {
     // Only remove it when not done already, otherwise we would remove a newer
     // version of the function with the same name.
     if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0)
        func_remove(fp);
 
-    if ((fp->uf_flags & FC_DEAD) == 0)
+    if ((fp->uf_flags & FC_DEAD) == 0 || force)
        vim_free(fp);
 }
 
@@ -1037,7 +1038,7 @@ func_free(ufunc_T *fp)
 func_clear_free(ufunc_T *fp, int force)
 {
     func_clear(fp, force);
-    func_free(fp);
+    func_free(fp, force);
 }
 
 
@@ -1664,7 +1665,7 @@ free_all_functions(void)
                    ++skipped;
                else
                {
-                   func_free(fp);
+                   func_free(fp, FALSE);
                    skipped = 0;
                    break;
                }
@@ -4392,8 +4393,6 @@ set_ref_in_functions(int copyID)
            fp = HI2UF(hi);
            if (!func_name_refcount(fp->uf_name))
                abort = abort || set_ref_in_func(NULL, fp, copyID);
-           else if (fp->uf_dfunc_idx >= 0)
-               abort = abort || set_ref_in_dfunc(fp, copyID);
        }
     }
     return abort;
@@ -4441,8 +4440,6 @@ set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID)
     {
        for (fc = fp->uf_scoped; fc != NULL; fc = fc->func->uf_scoped)
            abort = abort || set_ref_in_funccal(fc, copyID);
-       if (fp->uf_dfunc_idx >= 0)
-           abort = abort || set_ref_in_dfunc(fp, copyID);
     }
 
     vim_free(tofree);

diff --git a/src/version.c b/src/version.c
index ae0855405ccbe17f56b3e48b326e0ba3638995ad..2bccb328a80f5cf05414e3b2bd6d5e34d2434e48 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -746,6 +746,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    684,
 /**/
     683,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index befa6a74c89e27bb583036144f1363f69817d420..aac776af1cd10ed83f6aa5711a9e0cca7c20bd9c 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -259,21 +259,6 @@ struct isn_S {
     } isn_arg;
 };
 
-/*
- * Structure to hold the context of a compiled function, used by closures
- * defined in that function.
- */
-typedef struct funcstack_S
-{
-    garray_T   fs_ga;          // contains the stack, with:
-                               // - arguments
-                               // - frame
-                               // - local variables
-
-    int                fs_refcount;    // nr of closures referencing this funcstack
-    int                fs_copyID;      // for garray_T collection
-} funcstack_T;
-
 /*
  * Info about a function defined with :def.  Used in "def_functions".
  */
@@ -286,11 +271,6 @@ struct dfunc_S {
     isn_T      *df_instr;          // function body to be executed
     int                df_instr_count;
 
-    garray_T   *df_ectx_stack;     // where compiled closure finds local vars
-    int                df_ectx_frame;      // index of function frame in uf_ectx_stack
-    funcstack_T        *df_funcstack;      // copy of stack for closure, used after
-                                   // closure context function returns
-
     int                df_varcount;        // number of local variables
     int                df_closure_count;   // number of closures created
 };

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 2404282f8a7f5fcabfbb13dfa69f98932957df48..c74240ff2037f217b3b12da791959dcbea602b23 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -232,10 +232,6 @@ call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx)
     ectx->ec_instr = dfunc->df_instr;
     estack_push_ufunc(ETYPE_UFUNC, dfunc->df_ufunc, 1);
 
-    // used for closures
-    ectx->ec_outer_stack = dfunc->df_ectx_stack;
-    ectx->ec_outer_frame = dfunc->df_ectx_frame;
-
     // Decide where to start execution, handles optional arguments.
     init_instr_idx(ufunc, argcount, ectx);
 
@@ -269,7 +265,10 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
        tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
                                                   + dfunc->df_varcount + idx);
        if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial->pt_refcount > 1)
+       {
            closure_in_use = TRUE;
+           break;
+       }
     }
 
     if (closure_in_use)
@@ -315,15 +314,17 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
        {
            tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
                                                   + dfunc->df_varcount + idx);
-           if (tv->v_type == VAR_PARTIAL
-                                       && tv->vval.v_partial->pt_refcount > 1)
+           if (tv->v_type == VAR_PARTIAL)
            {
-               dfunc_T *pt_dfunc = ((dfunc_T *)def_functions.ga_data)
-                                  + tv->vval.v_partial->pt_func->uf_dfunc_idx;
-               ++funcstack->fs_refcount;
-               pt_dfunc->df_funcstack = funcstack;
-               pt_dfunc->df_ectx_stack = &funcstack->fs_ga;
-               pt_dfunc->df_ectx_frame = ectx->ec_frame_idx - top;
+               partial_T *partial = tv->vval.v_partial;
+
+               if (partial->pt_refcount > 1)
+               {
+                   ++funcstack->fs_refcount;
+                   partial->pt_funcstack = funcstack;
+                   partial->pt_ectx_stack = &funcstack->fs_ga;
+                   partial->pt_ectx_frame = ectx->ec_frame_idx - top;
+               }
            }
        }
     }
@@ -515,7 +516,15 @@ call_partial(typval_T *tv, int argcount, ectx_T *ectx)
        partial_T *pt = tv->vval.v_partial;
 
        if (pt->pt_func != NULL)
-           return call_ufunc(pt->pt_func, argcount, ectx, NULL);
+       {
+           int ret = call_ufunc(pt->pt_func, argcount, ectx, NULL);
+
+           // closure may need the function context where it was defined
+           ectx->ec_outer_stack = pt->pt_ectx_stack;
+           ectx->ec_outer_frame = pt->pt_ectx_frame;
+
+           return ret;
+       }
        name = pt->pt_name;
     }
     else if (tv->v_type == VAR_FUNC)
@@ -1434,8 +1443,8 @@ call_def_function(
 
                        // The closure needs to find arguments and local
                        // variables in the current stack.
-                       pt_dfunc->df_ectx_stack = &ectx.ec_stack;
-                       pt_dfunc->df_ectx_frame = ectx.ec_frame_idx;
+                       pt->pt_ectx_stack = &ectx.ec_stack;
+                       pt->pt_ectx_frame = ectx.ec_frame_idx;
 
                        // If this function returns and the closure is still
                        // used, we need to make a copy of the context
@@ -2676,25 +2685,5 @@ check_not_string(typval_T *tv)
     return OK;
 }
 
-/*
- * Mark items in a def function as used.
- */
-    int
-set_ref_in_dfunc(ufunc_T *ufunc, int copyID)
-{
-    dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ufunc->uf_dfunc_idx;
-    int            abort = FALSE;
-
-    if (dfunc->df_funcstack != NULL)
-    {
-       typval_T    *stack = dfunc->df_funcstack->fs_ga.ga_data;
-       int         idx;
-
-       for (idx = 0; idx < dfunc->df_funcstack->fs_ga.ga_len; ++idx)
-           abort = abort || set_ref_in_item(stack + idx, copyID, NULL, NULL);
-    }
-    return abort;
-}
-
 
 #endif // FEAT_EVAL