Patch 7.4.2142
Problem: Leaking memory when redefining a function.
Solution: Don't increment the function reference count when it's found by
name. Don't remove the wrong function from the hashtab. More
reference counting fixes.
Files: src/structs.h, src/userfunc.c
*** ../vim-7.4.2141/src/structs.h 2016-08-01 16:35:56.472496015 +0200
--- src/structs.h 2016-08-01 19:53:46.630962268 +0200
***************
*** 1327,1333 ****
#endif
scid_T uf_script_ID; /* ID of script where function was defined,
used for s: variables */
! int uf_refcount; /* for numbered function: reference
count */
funccall_T *uf_scoped; /* l: local variables for closure */
char_u uf_name[1]; /* name of function (actually longer); can
start with <SNR>123_ (<SNR> is K_SPECIAL
--- 1327,1333 ----
#endif
scid_T uf_script_ID; /* ID of script where function was defined,
used for s: variables */
! int uf_refcount; /* reference count, see
func_name_refcount() */
funccall_T *uf_scoped; /* l: local variables for closure */
char_u uf_name[1]; /* name of function (actually longer); can
start with <SNR>123_ (<SNR> is K_SPECIAL
***************
*** 1365,1373 ****
funccall_T *caller; /* calling function or NULL */
/* for closure */
! int fc_refcount;
int fc_copyID; /* for garbage collection */
! garray_T fc_funcs; /* list of ufunc_T* which refer this */
};
/*
--- 1365,1375 ----
funccall_T *caller; /* calling function or NULL */
/* for closure */
! int fc_refcount; /* number of user functions that
reference this
! * funccal */
int fc_copyID; /* for garbage collection */
! garray_T fc_funcs; /* list of ufunc_T* which keep a reference to
! * "func" */
};
/*
*** ../vim-7.4.2141/src/userfunc.c 2016-08-01 17:10:13.593214865 +0200
--- src/userfunc.c 2016-08-01 19:58:18.616522721 +0200
***************
*** 15,25 ****
#if defined(FEAT_EVAL) || defined(PROTO)
/* function flags */
! #define FC_ABORT 1 /* abort function on error */
! #define FC_RANGE 2 /* function accepts range */
! #define FC_DICT 4 /* Dict function, uses "self" */
! #define FC_CLOSURE 8 /* closure, uses outer scope variables */
! #define FC_DELETED 16 /* :delfunction used while uf_refcount
> 0 */
/* From user function to hashitem and back. */
#define UF2HIKEY(fp) ((fp)->uf_name)
--- 15,26 ----
#if defined(FEAT_EVAL) || defined(PROTO)
/* function flags */
! #define FC_ABORT 0x01 /* abort function on error */
! #define FC_RANGE 0x02 /* function accepts range */
! #define FC_DICT 0x04 /* Dict function, uses "self" */
! #define FC_CLOSURE 0x08 /* closure, uses outer scope variables */
! #define FC_DELETED 0x10 /* :delfunction used while uf_refcount > 0 */
! #define FC_REMOVED 0x20 /* function redefined while uf_refcount > 0 */
/* From user function to hashitem and back. */
#define UF2HIKEY(fp) ((fp)->uf_name)
***************
*** 178,191 ****
static int
register_closure(ufunc_T *fp)
{
! funccal_unref(fp->uf_scoped, NULL);
fp->uf_scoped = current_funccal;
current_funccal->fc_refcount++;
if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL)
return FAIL;
((ufunc_T **)current_funccal->fc_funcs.ga_data)
[current_funccal->fc_funcs.ga_len++] = fp;
- func_ptr_ref(current_funccal->func);
return OK;
}
--- 179,196 ----
static int
register_closure(ufunc_T *fp)
{
! 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;
((ufunc_T **)current_funccal->fc_funcs.ga_data)
[current_funccal->fc_funcs.ga_len++] = fp;
return OK;
}
***************
*** 1024,1083 ****
/*
* Unreference "fc": decrement the reference count and free it when it
! * becomes zero. If "fp" is not NULL, "fp" is detached from "fc".
*/
static void
funccal_unref(funccall_T *fc, ufunc_T *fp)
{
funccall_T **pfc;
int i;
- int freed = FALSE;
if (fc == NULL)
return;
! if (--fc->fc_refcount <= 0)
! {
for (pfc = &previous_funccal; *pfc != NULL; pfc = &(*pfc)->caller)
{
if (fc == *pfc)
{
! if (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)
! {
! *pfc = fc->caller;
! free_funccal(fc, TRUE);
! freed = TRUE;
! }
! break;
}
}
! }
! if (!freed)
{
! func_ptr_unref(fc->func);
!
! if (fp != NULL)
! for (i = 0; i < fc->fc_funcs.ga_len; ++i)
! {
! if (((ufunc_T **)(fc->fc_funcs.ga_data))[i] == fp)
! ((ufunc_T **)(fc->fc_funcs.ga_data))[i] = NULL;
! }
}
}
/*
* Remove the function from the function hashtable. If the function was
* deleted while it still has references this was already done.
*/
! static void
func_remove(ufunc_T *fp)
{
hashitem_T *hi = hash_find(&func_hashtab, UF2HIKEY(fp));
if (!HASHITEM_EMPTY(hi))
hash_remove(&func_hashtab, hi);
}
/*
--- 1029,1084 ----
/*
* 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;
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)
{
! *pfc = fc->caller;
! free_funccal(fc, TRUE);
! return;
}
}
! 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;
! }
}
}
/*
* Remove the function from the function hashtable. If the function was
* deleted while it still has references this was already done.
+ * Return TRUE if the entry was deleted, FALSE if it wasn't found.
*/
! static int
func_remove(ufunc_T *fp)
{
hashitem_T *hi = hash_find(&func_hashtab, UF2HIKEY(fp));
if (!HASHITEM_EMPTY(hi))
+ {
hash_remove(&func_hashtab, hi);
+ return TRUE;
+ }
+ return FALSE;
}
/*
***************
*** 1094,1100 ****
vim_free(fp->uf_tml_total);
vim_free(fp->uf_tml_self);
#endif
! func_remove(fp);
funccal_unref(fp->uf_scoped, fp);
--- 1095,1104 ----
vim_free(fp->uf_tml_total);
vim_free(fp->uf_tml_self);
#endif
! /* only remove it when not done already, otherwise we would remove a newer
! * version of the function */
! if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0)
! func_remove(fp);
funccal_unref(fp->uf_scoped, fp);
***************
*** 2158,2163 ****
--- 2162,2168 ----
/* This function is referenced somewhere, don't redefine it but
* create a new one. */
--fp->uf_refcount;
+ fp->uf_flags |= FC_REMOVED;
fp = NULL;
overwrite = TRUE;
}
***************
*** 2651,2656 ****
--- 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
***************
*** 2705,2723 ****
}
else
{
! /* Normal functions (not numbered functions and lambdas) have a
* refcount of 1 for the entry in the hashtable. When deleting
! * them and the refcount is more than one, it should be kept.
! * Numbered functions and lambdas snould be kept if the refcount is
* one or more. */
! if (fp->uf_refcount > (isdigit(fp->uf_name[0])
! || fp->uf_name[0] == '<' ? 0 : 1))
{
/* Function is still referenced somewhere. Don't free it but
* do remove it from the hashtable. */
! func_remove(fp);
fp->uf_flags |= FC_DELETED;
- fp->uf_refcount--;
}
else
func_free(fp);
--- 2724,2741 ----
}
else
{
! /* A normal function (not a numbered function or lambda) has a
* refcount of 1 for the entry in the hashtable. When deleting
! * it and the refcount is more than one, it should be kept.
! * A numbered function and lambda snould be kept if the refcount is
* one or more. */
! if (fp->uf_refcount > (func_name_refcount(fp->uf_name) ? 0 : 1))
{
/* Function is still referenced somewhere. Don't free it but
* do remove it from the hashtable. */
! if (func_remove(fp))
! fp->uf_refcount--;
fp->uf_flags |= FC_DELETED;
}
else
func_free(fp);
***************
*** 2734,2740 ****
{
ufunc_T *fp = NULL;
! if (name == NULL)
return;
fp = find_func(name);
if (fp == NULL && isdigit(*name))
--- 2752,2758 ----
{
ufunc_T *fp = NULL;
! if (name == NULL || !func_name_refcount(name))
return;
fp = find_func(name);
if (fp == NULL && isdigit(*name))
***************
*** 2777,2783 ****
{
ufunc_T *fp;
! if (name == NULL)
return;
fp = find_func(name);
if (fp != NULL)
--- 2795,2801 ----
{
ufunc_T *fp;
! if (name == NULL || !func_name_refcount(name))
return;
fp = find_func(name);
if (fp != NULL)
*** ../vim-7.4.2141/src/version.c 2016-08-01 17:10:13.601214790 +0200
--- src/version.c 2016-08-01 20:44:36.543597750 +0200
***************
*** 765,766 ****
--- 765,768 ----
{ /* Add new patch number below this line */
+ /**/
+ 2142,
/**/
--
(letter from Mark to Mike, about the film's probable certificate)
I would like to get back to the Censor and agree to lose the shits, take
the odd Jesus Christ out and lose Oh fuck off, but to retain 'fart in
your general direction', 'castanets of your testicles' and 'oral sex'
and ask him for an 'A' rating on that basis.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
/// 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.
