Patch 7.4.2143
Problem: A funccal is garbage collected while it can still be used.
Solution: Set copyID in all referenced functions. Do not list lambda
functions with ":function".
Files: src/userfunc.c, src/proto/userfunc.pro, src/eval.c,
src/testdir/test_lambda.vim
*** ../vim-7.4.2142/src/userfunc.c 2016-08-01 20:46:20.610667730 +0200
--- src/userfunc.c 2016-08-01 22:38:13.459038004 +0200
***************
*** 65,71 ****
# endif
prof_self_cmp(const void *s1, const void *s2);
#endif
! static void funccal_unref(funccall_T *fc, ufunc_T *fp);
void
func_init()
--- 65,71 ----
# endif
prof_self_cmp(const void *s1, const void *s2);
#endif
! static void funccal_unref(funccall_T *fc, ufunc_T *fp, int force);
void
func_init()
***************
*** 182,191 ****
if (fp->uf_scoped == current_funccal)
/* no change */
return OK;
! funccal_unref(fp->uf_scoped, fp);
fp->uf_scoped = current_funccal;
current_funccal->fc_refcount++;
- func_ptr_ref(current_funccal->func);
if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL)
return FAIL;
--- 182,190 ----
if (fp->uf_scoped == current_funccal)
/* no change */
return OK;
! funccal_unref(fp->uf_scoped, fp, FALSE);
fp->uf_scoped = current_funccal;
current_funccal->fc_refcount++;
if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL)
return FAIL;
***************
*** 603,616 ****
{
ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
! if (fp != NULL)
! {
! /* Function may have been redefined and point to another
! * funccall_T, don't clear it then. */
! if (fp->uf_scoped == fc)
! fp->uf_scoped = NULL;
! func_ptr_unref(fc->func);
! }
}
ga_clear(&fc->fc_funcs);
--- 602,613 ----
{
ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
! /* When garbage collecting a funccall_T may be freed before the
! * function that references it, clear its uf_scoped field.
! * The function may have been redefined and point to another
! * funccall_T, don't clear it then. */
! if (fp != NULL && fp->uf_scoped == fc)
! fp->uf_scoped = NULL;
}
ga_clear(&fc->fc_funcs);
***************
*** 1030,1038 ****
/*
* Unreference "fc": decrement the reference count and free it when it
* becomes zero. "fp" is detached from "fc".
*/
static void
! funccal_unref(funccall_T *fc, ufunc_T *fp)
{
funccall_T **pfc;
int i;
--- 1027,1036 ----
/*
* Unreference "fc": decrement the reference count and free it when it
* becomes zero. "fp" is detached from "fc".
+ * When "force" is TRUE we are exiting.
*/
static void
! funccal_unref(funccall_T *fc, ufunc_T *fp, int force)
{
funccall_T **pfc;
int i;
***************
*** 1040,1049 ****
if (fc == NULL)
return;
! if (--fc->fc_refcount <= 0
! && fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT
&& fc->l_vars.dv_refcount == DO_NOT_FREE_CNT
! && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT)
for (pfc = &previous_funccal; *pfc != NULL; pfc = &(*pfc)->caller)
{
if (fc == *pfc)
--- 1038,1047 ----
if (fc == NULL)
return;
! if (--fc->fc_refcount <= 0 && (force || (
! fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT
&& fc->l_vars.dv_refcount == DO_NOT_FREE_CNT
! && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT)))
for (pfc = &previous_funccal; *pfc != NULL; pfc = &(*pfc)->caller)
{
if (fc == *pfc)
***************
*** 1054,1066 ****
}
}
for (i = 0; i < fc->fc_funcs.ga_len; ++i)
- {
if (((ufunc_T **)(fc->fc_funcs.ga_data))[i] == fp)
- {
- func_ptr_unref(fc->func);
((ufunc_T **)(fc->fc_funcs.ga_data))[i] = NULL;
- }
- }
}
/*
--- 1052,1059 ----
***************
*** 1083,1091 ****
/*
* Free a function and remove it from the list of functions.
*/
static void
! func_free(ufunc_T *fp)
{
/* clear this function */
ga_clear_strings(&(fp->uf_args));
--- 1076,1085 ----
/*
* Free a function and remove it from the list of functions.
+ * When "force" is TRUE we are exiting.
*/
static void
! func_free(ufunc_T *fp, int force)
{
/* clear this function */
ga_clear_strings(&(fp->uf_args));
***************
*** 1100,1106 ****
if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0)
func_remove(fp);
! funccal_unref(fp->uf_scoped, fp);
vim_free(fp);
}
--- 1094,1100 ----
if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0)
func_remove(fp);
! funccal_unref(fp->uf_scoped, fp, force);
vim_free(fp);
}
***************
*** 1117,1123 ****
for (hi = func_hashtab.ht_array; ; ++hi)
if (!HASHITEM_EMPTY(hi))
{
! func_free(HI2UF(hi));
break;
}
hash_clear(&func_hashtab);
--- 1111,1117 ----
for (hi = func_hashtab.ht_array; ; ++hi)
if (!HASHITEM_EMPTY(hi))
{
! func_free(HI2UF(hi), TRUE);
break;
}
hash_clear(&func_hashtab);
***************
*** 1331,1337 ****
if (--fp->uf_calls <= 0 && fp->uf_refcount <= 0)
/* Function was unreferenced while being used, free it
* now. */
! func_free(fp);
if (did_save_redo)
restoreRedobuff();
restore_search_patterns();
--- 1325,1331 ----
if (--fp->uf_calls <= 0 && fp->uf_refcount <= 0)
/* Function was unreferenced while being used, free it
* now. */
! func_free(fp, FALSE);
if (did_save_redo)
restoreRedobuff();
restore_search_patterns();
***************
*** 1675,1680 ****
--- 1669,1688 ----
}
/*
+ * There are two kinds of function names:
+ * 1. ordinary names, function defined with :function
+ * 2. numbered functions and lambdas
+ * For the first we only count the name stored in func_hashtab as a reference,
+ * using function() does not count as a reference, because the function is
+ * looked up by name.
+ */
+ static int
+ func_name_refcount(char_u *name)
+ {
+ return isdigit(*name) || *name == '<';
+ }
+
+ /*
* ":function"
*/
void
***************
*** 1721,1727 ****
{
--todo;
fp = HI2UF(hi);
! if (!isdigit(*fp->uf_name))
list_func_head(fp, FALSE);
}
}
--- 1729,1735 ----
{
--todo;
fp = HI2UF(hi);
! if (!func_name_refcount(fp->uf_name))
list_func_head(fp, FALSE);
}
}
***************
*** 2656,2675 ****
#endif /* FEAT_CMDL_COMPL */
/*
- * There are two kinds of function names:
- * 1. ordinary names, function defined with :function
- * 2. numbered functions and lambdas
- * For the first we only count the name stored in func_hashtab as a reference,
- * using function() does not count as a reference, because the function is
- * looked up by name.
- */
- static int
- func_name_refcount(char_u *name)
- {
- return isdigit(*name) || *name == '<';
- }
-
- /*
* ":delfunction {name}"
*/
void
--- 2664,2669 ----
***************
*** 2738,2744 ****
fp->uf_flags |= FC_DELETED;
}
else
! func_free(fp);
}
}
}
--- 2732,2738 ----
fp->uf_flags |= FC_DELETED;
}
else
! func_free(fp, FALSE);
}
}
}
***************
*** 2767,2773 ****
/* Only delete it when it's not being used. Otherwise it's done
* when "uf_calls" becomes zero. */
if (fp->uf_calls == 0)
! func_free(fp);
}
}
--- 2761,2767 ----
/* Only delete it when it's not being used. Otherwise it's done
* when "uf_calls" becomes zero. */
if (fp->uf_calls == 0)
! func_free(fp, FALSE);
}
}
***************
*** 2783,2789 ****
/* Only delete it when it's not being used. Otherwise it's done
* when "uf_calls" becomes zero. */
if (fp->uf_calls == 0)
! func_free(fp);
}
}
--- 2777,2783 ----
/* Only delete it when it's not being used. Otherwise it's done
* when "uf_calls" becomes zero. */
if (fp->uf_calls == 0)
! func_free(fp, FALSE);
}
}
***************
*** 3676,3681 ****
--- 3670,3690 ----
return abort;
}
+ static int
+ set_ref_in_funccal(funccall_T *fc, int copyID)
+ {
+ int abort = FALSE;
+
+ if (fc->fc_copyID != copyID)
+ {
+ fc->fc_copyID = copyID;
+ abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID, NULL);
+ abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL);
+ abort = abort || set_ref_in_func(NULL, fc->func, copyID);
+ }
+ return abort;
+ }
+
/*
* Set "copyID" in all local vars and arguments in the call stack.
*/
***************
*** 3686,3695 ****
funccall_T *fc;
for (fc = current_funccal; fc != NULL; fc = fc->caller)
{
! fc->fc_copyID = copyID;
! abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID, NULL);
! abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL);
}
return abort;
}
--- 3695,3725 ----
funccall_T *fc;
for (fc = current_funccal; fc != NULL; fc = fc->caller)
+ abort = abort || set_ref_in_funccal(fc, copyID);
+ return abort;
+ }
+
+ /*
+ * Set "copyID" in all functions available by name.
+ */
+ int
+ set_ref_in_functions(int copyID)
+ {
+ int todo;
+ hashitem_T *hi = NULL;
+ int abort = FALSE;
+ ufunc_T *fp;
+
+ todo = (int)func_hashtab.ht_used;
+ for (hi = func_hashtab.ht_array; todo > 0 && !got_int; ++hi)
{
! if (!HASHITEM_EMPTY(hi))
! {
! --todo;
! fp = HI2UF(hi);
! if (!func_name_refcount(fp->uf_name))
! abort = abort || set_ref_in_func(NULL, fp, copyID);
! }
}
return abort;
}
***************
*** 3711,3719 ****
/*
* Mark all lists and dicts referenced through function "name" with "copyID".
- * "list_stack" is used to add lists to be marked. Can be NULL.
- * "ht_stack" is used to add hashtabs to be marked. Can be NULL.
- *
* Returns TRUE if setting references failed somehow.
*/
int
--- 3741,3746 ----
***************
*** 3725,3730 ****
--- 3752,3758 ----
char_u fname_buf[FLEN_FIXED + 1];
char_u *tofree = NULL;
char_u *fname;
+ int abort = FALSE;
if (name == NULL && fp_in == NULL)
return FALSE;
***************
*** 3737,3753 ****
if (fp != NULL)
{
for (fc = fp->uf_scoped; fc != NULL; fc = fc->func->uf_scoped)
! {
! if (fc->fc_copyID != copyID)
! {
! fc->fc_copyID = copyID;
! set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID, NULL);
! set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL);
! }
! }
}
vim_free(tofree);
! return FALSE;
}
#endif /* FEAT_EVAL */
--- 3765,3774 ----
if (fp != NULL)
{
for (fc = fp->uf_scoped; fc != NULL; fc = fc->func->uf_scoped)
! abort = abort || set_ref_in_funccal(fc, copyID);
}
vim_free(tofree);
! return abort;
}
#endif /* FEAT_EVAL */
*** ../vim-7.4.2142/src/proto/userfunc.pro 2016-08-01 17:10:13.593214865
+0200
--- src/proto/userfunc.pro 2016-08-01 21:43:03.616306718 +0200
***************
*** 54,59 ****
--- 54,60 ----
dictitem_T *find_var_in_scoped_ht(char_u *name, int no_autoload);
int set_ref_in_previous_funccal(int copyID);
int set_ref_in_call_stack(int copyID);
+ int set_ref_in_functions(int copyID);
int set_ref_in_func_args(int copyID);
int set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID);
/* vim: set ft=c : */
*** ../vim-7.4.2142/src/eval.c 2016-08-01 17:10:13.601214790 +0200
--- src/eval.c 2016-08-01 22:20:55.712231023 +0200
***************
*** 5304,5309 ****
--- 5304,5312 ----
/* function-local variables */
abort = abort || set_ref_in_call_stack(copyID);
+ /* named functions (matters for closures) */
+ abort = abort || set_ref_in_functions(copyID);
+
/* function call arguments, if v:testing is set. */
abort = abort || set_ref_in_func_args(copyID);
*** ../vim-7.4.2142/src/testdir/test_lambda.vim 2016-08-01 16:29:42.516009792
+0200
--- src/testdir/test_lambda.vim 2016-08-01 22:26:44.049146492 +0200
***************
*** 270,272 ****
--- 270,286 ----
delfunc LambdaFoo
delfunc LambdaBar
endfunc
+
+ func Test_named_function_closure()
+ func! Afoo()
+ let x = 14
+ func! s:Abar() closure
+ return x
+ endfunc
+ call assert_equal(14, s:Abar())
+ endfunc
+ call Afoo()
+ call assert_equal(14, s:Abar())
+ call test_garbagecollect_now()
+ call assert_equal(14, s:Abar())
+ endfunc
*** ../vim-7.4.2142/src/version.c 2016-08-01 20:46:20.614667695 +0200
--- src/version.c 2016-08-01 20:57:19.548765670 +0200
***************
*** 765,766 ****
--- 765,768 ----
{ /* Add new patch number below this line */
+ /**/
+ 2143,
/**/
--
A fool must search for a greater fool to find admiration.
/// 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].
For more options, visit https://groups.google.com/d/optout.