thac0/vim
1
0
Fork 0
forked from aniani/vim
Code Activity

patch 8.2.0684: Vim9: memory leak when using lambda

Browse Source 
Problem:    Vim9: memory leak when using lambda.
Solution:   Move the funccal context to the partial. Free the function when
            exiting.
This commit is contained in:
Bram Moolenaar
2020-05-03 15:38:16 +02:00
parent 5adc55cb74
commit f7779c63d4
7 changed files with 83 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/eval.c
View File

@@ -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

23
src/structs.h
View File

@@ -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"
};

10
src/testdir/test_vim9_func.vim
View File

@@ -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

13
src/userfunc.c
View File

@@ -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);

2
src/version.c
View File

@@ -746,6 +746,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
684,
/**/
683,
/**/

20
src/vim9.h
View File

@@ -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
};

59
src/vim9execute.c
View File

@@ -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