To: vim_dev@googlegroups.com Subject: Patch 8.2.0684 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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 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 -- Bram@Moolenaar.net -- 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 ///