Patch 8.2.0684
Problem: Vim9: memory leak when using lambda.
Solution: Move the funccal context to the partial. Free the function when
exiting.
Files: src/vim9.h, src/structs.h, src/vim9execute.c, src/userfunc.c,
src/eval.c, src/testdir/test_vim9_func.vim
*** ../vim-8.2.0683/src/vim9.h 2020-05-02 17:52:38.404147677 +0200
--- src/vim9.h 2020-05-03 15:20:52.927560314 +0200
***************
*** 260,280 ****
};
/*
- * 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 {
--- 260,265 ----
***************
*** 286,296 ****
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
};
--- 271,276 ----
*** ../vim-8.2.0683/src/structs.h 2020-05-02 17:52:38.404147677 +0200
--- src/structs.h 2020-05-03 15:21:07.199509872 +0200
***************
*** 1777,1782 ****
--- 1777,1797 ----
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,1793 ****
--- 1801,1816 ----
// 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"
};
*** ../vim-8.2.0683/src/vim9execute.c 2020-05-02 23:12:54.722410350 +0200
--- src/vim9execute.c 2020-05-03 15:34:45.880455439 +0200
***************
*** 232,241 ****
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);
--- 232,237 ----
***************
*** 269,275 ****
--- 265,274 ----
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,329 ****
{
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)
{
! 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;
}
}
}
--- 314,330 ----
{
tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
+ dfunc->df_varcount + idx);
! if (tv->v_type == VAR_PARTIAL)
{
! 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,521 ****
partial_T *pt = tv->vval.v_partial;
if (pt->pt_func != NULL)
! return call_ufunc(pt->pt_func, argcount, ectx, NULL);
name = pt->pt_name;
}
else if (tv->v_type == VAR_FUNC)
--- 516,530 ----
partial_T *pt = tv->vval.v_partial;
if (pt->pt_func != 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,1441 ****
// 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;
// If this function returns and the closure is still
// used, we need to make a copy of the context
--- 1443,1450 ----
// The closure needs to find arguments and local
// variables in the current stack.
! 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,2700 ****
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
--- 2685,2689 ----
*** ../vim-8.2.0683/src/userfunc.c 2020-05-02 23:12:54.722410350 +0200
--- src/userfunc.c 2020-05-03 15:18:18.644106604 +0200
***************
*** 1016,1031 ****
/*
* Free a function and remove it from the list of functions. Does not free
* what a function contains, call func_clear() first.
*/
static void
! func_free(ufunc_T *fp)
{
// 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)
vim_free(fp);
}
--- 1016,1032 ----
/*
* 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, 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 || force)
vim_free(fp);
}
***************
*** 1037,1043 ****
func_clear_free(ufunc_T *fp, int force)
{
func_clear(fp, force);
! func_free(fp);
}
--- 1038,1044 ----
func_clear_free(ufunc_T *fp, int force)
{
func_clear(fp, force);
! func_free(fp, force);
}
***************
*** 1664,1670 ****
++skipped;
else
{
! func_free(fp);
skipped = 0;
break;
}
--- 1665,1671 ----
++skipped;
else
{
! func_free(fp, FALSE);
skipped = 0;
break;
}
***************
*** 4392,4399 ****
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;
--- 4393,4398 ----
***************
*** 4441,4448 ****
{
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);
--- 4440,4445 ----
*** ../vim-8.2.0683/src/eval.c 2020-04-30 22:29:36.622024155 +0200
--- src/eval.c 2020-05-03 15:34:35.976491769 +0200
***************
*** 3703,3708 ****
--- 3703,3726 ----
}
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,4341 ****
--- 4354,4368 ----
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
*** ../vim-8.2.0683/src/testdir/test_vim9_func.vim 2020-05-02
23:12:54.726410335 +0200
--- src/testdir/test_vim9_func.vim 2020-05-03 14:55:31.433221437 +0200
***************
*** 680,692 ****
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
--- 680,685 ----
***************
*** 696,702 ****
enddef
def Test_closure_two_indirect_refs()
! MakeTwoRefs_2()
assert_equal('some', ReadRef(g:Read))
ExtendRef(g:Extend, 'more')
assert_equal('some more', ReadRef(g:Read))
--- 689,695 ----
enddef
def Test_closure_two_indirect_refs()
! MakeTwoRefs()
assert_equal('some', ReadRef(g:Read))
ExtendRef(g:Extend, 'more')
assert_equal('some more', ReadRef(g:Read))
***************
*** 707,710 ****
--- 700,704 ----
unlet g:Read
enddef
+
" vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
*** ../vim-8.2.0683/src/version.c 2020-05-02 23:12:54.726410335 +0200
--- src/version.c 2020-05-03 14:45:24.119649418 +0200
***************
*** 748,749 ****
--- 748,751 ----
{ /* Add new patch number below this line */
+ /**/
+ 684,
/**/
--
Just remember...if the world didn't suck, we'd all fall off.
/// Bram Moolenaar -- [email protected] -- 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 ///
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/vim_dev/202005031338.043Dcpik010635%40masaka.moolenaar.net.
