To: vim_dev@googlegroups.com Subject: Patch 8.2.2391 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2391 Problem: Memory leak when creating a global function with closure. Solution: Create a separate partial for every instantiated function. Files: src/userfunc.c, src/vim9execute.c *** ../vim-8.2.2390/src/userfunc.c 2021-01-16 18:09:48.017277750 +0100 --- src/userfunc.c 2021-01-22 20:32:06.826000087 +0100 *************** *** 1352,1359 **** --- 1352,1364 ---- VIM_CLEAR(fp->uf_block_ids); VIM_CLEAR(fp->uf_va_name); clear_type_list(&fp->uf_type_list); + + // Increment the refcount of this function to avoid it being freed + // recursively when the partial is freed. + fp->uf_refcount += 3; partial_unref(fp->uf_partial); fp->uf_partial = NULL; + fp->uf_refcount -= 3; #ifdef FEAT_LUA if (fp->uf_cb_free != NULL) *************** *** 1446,1455 **** return FAIL; } - // TODO: handle ! to overwrite fp = find_func(global, TRUE, NULL); if (fp != NULL) { semsg(_(e_funcexts), global); return FAIL; } --- 1451,1460 ---- return FAIL; } fp = find_func(global, TRUE, NULL); if (fp != NULL) { + // TODO: handle ! to overwrite semsg(_(e_funcexts), global); return FAIL; } *************** *** 1501,1508 **** // the referenced dfunc_T is now used one more time link_def_function(fp); ! // Create a partial to store the context of the function, if not done ! // already. if ((ufunc->uf_flags & FC_CLOSURE) && ufunc->uf_partial == NULL) { partial_T *pt = ALLOC_CLEAR_ONE(partial_T); --- 1506,1514 ---- // the referenced dfunc_T is now used one more time link_def_function(fp); ! // Create a partial to store the context of the function where it was ! // instantiated. Only needs to be done once. Do this on the original ! // function, "dfunc->df_ufunc" will point to it. if ((ufunc->uf_flags & FC_CLOSURE) && ufunc->uf_partial == NULL) { partial_T *pt = ALLOC_CLEAR_ONE(partial_T); *************** *** 1510,1523 **** if (pt == NULL) goto failed; if (fill_partial_and_closure(pt, ufunc, ectx) == FAIL) goto failed; ufunc->uf_partial = pt; ! --pt->pt_refcount; // not referenced here yet ! } ! if (ufunc->uf_partial != NULL) ! { ! fp->uf_partial = ufunc->uf_partial; ! ++fp->uf_partial->pt_refcount; } return OK; --- 1516,1527 ---- if (pt == NULL) goto failed; if (fill_partial_and_closure(pt, ufunc, ectx) == FAIL) + { + vim_free(pt); goto failed; + } ufunc->uf_partial = pt; ! --pt->pt_refcount; // not actually referenced here } return OK; *************** *** 4243,4265 **** #endif internal_error("func_unref()"); } ! if (fp != NULL && --fp->uf_refcount <= 0) ! { ! // Only delete it when it's not being used. Otherwise it's done ! // when "uf_calls" becomes zero. ! if (fp->uf_calls == 0) ! func_clear_free(fp, FALSE); ! } } /* * Unreference a Function: decrement the reference count and free it when it * becomes zero. */ void func_ptr_unref(ufunc_T *fp) { ! if (fp != NULL && --fp->uf_refcount <= 0) { // Only delete it when it's not being used. Otherwise it's done // when "uf_calls" becomes zero. --- 4247,4267 ---- #endif internal_error("func_unref()"); } ! func_ptr_unref(fp); } /* * Unreference a Function: decrement the reference count and free it when it * becomes zero. + * Also when it becomes one and uf_partial points to the function. */ void func_ptr_unref(ufunc_T *fp) { ! if (fp != NULL && (--fp->uf_refcount <= 0 ! || (fp->uf_refcount == 1 && fp->uf_partial != NULL ! && fp->uf_partial->pt_refcount <= 1 ! && fp->uf_partial->pt_func == fp))) { // Only delete it when it's not being used. Otherwise it's done // when "uf_calls" becomes zero. *** ../vim-8.2.2390/src/vim9execute.c 2021-01-22 17:51:02.762771043 +0100 --- src/vim9execute.c 2021-01-22 20:39:53.008839344 +0100 *************** *** 263,269 **** } ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount; ! if (pt != NULL || ufunc->uf_partial != NULL || ufunc->uf_flags & FC_CLOSURE) { outer_T *outer = ALLOC_CLEAR_ONE(outer_T); --- 263,270 ---- } ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount; ! if (pt != NULL || ufunc->uf_partial != NULL ! || (ufunc->uf_flags & FC_CLOSURE)) { outer_T *outer = ALLOC_CLEAR_ONE(outer_T); *************** *** 1062,1068 **** pt->pt_func = ufunc; pt->pt_refcount = 1; ! if (pt->pt_func->uf_flags & FC_CLOSURE) { dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ectx->ec_dfunc_idx; --- 1063,1069 ---- pt->pt_func = ufunc; pt->pt_refcount = 1; ! if (ufunc->uf_flags & FC_CLOSURE) { dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + ectx->ec_dfunc_idx; *************** *** 1093,1099 **** ++pt->pt_refcount; ++ectx->ec_funcrefs.ga_len; } ! ++pt->pt_func->uf_refcount; return OK; } --- 1094,1100 ---- ++pt->pt_refcount; ++ectx->ec_funcrefs.ga_len; } ! ++ufunc->uf_refcount; return OK; } *************** *** 1243,1266 **** ectx.ec_frame_idx = ectx.ec_stack.ga_len; initial_frame_idx = ectx.ec_frame_idx; - if (partial != NULL || ufunc->uf_partial != NULL) { ! ectx.ec_outer = ALLOC_CLEAR_ONE(outer_T); ! if (ectx.ec_outer == NULL) ! goto failed_early; ! if (partial != NULL) ! { ! if (partial->pt_outer.out_stack == NULL && current_ectx != NULL) { ! if (current_ectx->ec_outer != NULL) ! *ectx.ec_outer = *current_ectx->ec_outer; } else ! *ectx.ec_outer = partial->pt_outer; } - else - *ectx.ec_outer = ufunc->uf_partial->pt_outer; - ectx.ec_outer->out_up_is_copy = TRUE; } // dummy frame entries --- 1244,1275 ---- ectx.ec_frame_idx = ectx.ec_stack.ga_len; initial_frame_idx = ectx.ec_frame_idx; { ! dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) ! + ufunc->uf_dfunc_idx; ! ufunc_T *base_ufunc = dfunc->df_ufunc; ! ! // "uf_partial" is on the ufunc that "df_ufunc" points to, as is done ! // by copy_func(). ! if (partial != NULL || base_ufunc->uf_partial != NULL) ! { ! ectx.ec_outer = ALLOC_CLEAR_ONE(outer_T); ! if (ectx.ec_outer == NULL) ! goto failed_early; ! if (partial != NULL) { ! if (partial->pt_outer.out_stack == NULL && current_ectx != NULL) ! { ! if (current_ectx->ec_outer != NULL) ! *ectx.ec_outer = *current_ectx->ec_outer; ! } ! else ! *ectx.ec_outer = partial->pt_outer; } else ! *ectx.ec_outer = base_ufunc->uf_partial->pt_outer; ! ectx.ec_outer->out_up_is_copy = TRUE; } } // dummy frame entries *** ../vim-8.2.2390/src/version.c 2021-01-22 17:51:02.766771030 +0100 --- src/version.c 2021-01-22 20:43:34.500306834 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2391, /**/ -- hundred-and-one symptoms of being an internet addict: 209. Your house stinks because you haven't cleaned it in a week. /// 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 ///