Mercurial > vim

changeset 20257:683c2da4982b v8.2.0684

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
patch 8.2.0684: Vim9: memory leak when using lambda Commit: https://github.com/vim/vim/commit/f7779c63d4fe531e2483502d4441f24802342768 Author: Bram Moolenaar <Bram@vim.org> Date: Sun May 3 15:38:16 2020 +0200 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.
author Bram Moolenaar <Bram@vim.org>
date Sun, 03 May 2020 15:45:05 +0200
parents e08857045ec1
children ed616e60438d
files src/eval.c src/structs.h src/testdir/test_vim9_func.vim src/userfunc.c src/version.c src/vim9.h src/vim9execute.c
diffstat 7 files changed, 83 insertions(+), 71 deletions(-) [+]
line wrap: on
line diff
--- 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
--- 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"
 };
 
--- 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: st
 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
--- 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 *f
     {
 	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);
--- a/src/version.c
+++ b/src/version.c
@@ -747,6 +747,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    684,
+/**/
     683,
 /**/
     682,
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -260,21 +260,6 @@ struct isn_S {
 };
 
 /*
- * 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".
  */
 struct dfunc_S {
@@ -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
 };
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -232,10 +232,6 @@ call_dfunc(int cdf_idx, int argcount_arg
     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 
 	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 
 	{
 	    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,
 	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