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".

diff --git a/src/eval.c b/src/eval.c
index 1adbabc082ec8f7b037bfd362a552b0ede5d4773..45b8f07c389c670f835b64ed1234f588d63c49ff 100644 (file)
--- a/src/eval.c
+++ b/src/eval.c
@@ -5304,6 +5304,9 @@ garbage_collect(int testing)
     /* 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);
 

diff --git a/src/proto/userfunc.pro b/src/proto/userfunc.pro
index 25620f16d30a3931464ab9c8ace71fbb102c74bf..3149ec50a23ea8ec58fbfcacc61383690fe4a07f 100644 (file)
--- a/src/proto/userfunc.pro
+++ b/src/proto/userfunc.pro
@@ -54,6 +54,7 @@ hashitem_T *find_hi_in_scoped_ht(char_u *name, hashtab_T **pht);
 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 : */

diff --git a/src/testdir/test_lambda.vim b/src/testdir/test_lambda.vim
index 1df1f1c3e3d3f6f13dfdb61d0a547c831b2b0921..901d535e809fa79cf0be9b897480c9a1310e699f 100644 (file)
--- a/src/testdir/test_lambda.vim
+++ b/src/testdir/test_lambda.vim
@@ -270,3 +270,17 @@ func Test_closure_refcount()
   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

diff --git a/src/userfunc.c b/src/userfunc.c
index e08e53a78fb857c635923deaadc24144b3fc576a..065425965c7920e46f3ba64eeadd35708b72181e 100644 (file)
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -65,7 +65,7 @@ static int
 # endif
        prof_self_cmp(const void *s1, const void *s2);
 #endif
-static void funccal_unref(funccall_T *fc, ufunc_T *fp);
+static void funccal_unref(funccall_T *fc, ufunc_T *fp, int force);
 
     void
 func_init()
@@ -182,10 +182,9 @@ register_closure(ufunc_T *fp)
     if (fp->uf_scoped == current_funccal)
        /* no change */
        return OK;
-    funccal_unref(fp->uf_scoped, fp);
+    funccal_unref(fp->uf_scoped, fp, FALSE);
     fp->uf_scoped = current_funccal;
     current_funccal->fc_refcount++;
-    func_ptr_ref(current_funccal->func);
 
     if (ga_grow(&current_funccal->fc_funcs, 1) == FAIL)
        return FAIL;
@@ -603,14 +602,12 @@ free_funccal(
     {
        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);
-       }
+       /* 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,9 +1027,10 @@ call_user_func(
 /*
  * 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)
+funccal_unref(funccall_T *fc, ufunc_T *fp, int force)
 {
     funccall_T **pfc;
     int                i;
@@ -1040,10 +1038,10 @@ funccal_unref(funccall_T *fc, ufunc_T *fp)
     if (fc == NULL)
        return;
 
-    if (--fc->fc_refcount <= 0
-               && fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT
+    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)
+               && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT)))
        for (pfc = &previous_funccal; *pfc != NULL; pfc = &(*pfc)->caller)
        {
            if (fc == *pfc)
@@ -1054,13 +1052,8 @@ funccal_unref(funccall_T *fc, ufunc_T *fp)
            }
        }
     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;
-       }
-    }
 }
 
 /*
@@ -1083,9 +1076,10 @@ func_remove(ufunc_T *fp)
 
 /*
  * 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)
+func_free(ufunc_T *fp, int force)
 {
     /* clear this function */
     ga_clear_strings(&(fp->uf_args));
@@ -1100,7 +1094,7 @@ func_free(ufunc_T *fp)
     if ((fp->uf_flags & (FC_DELETED | FC_REMOVED)) == 0)
        func_remove(fp);
 
-    funccal_unref(fp->uf_scoped, fp);
+    funccal_unref(fp->uf_scoped, fp, force);
 
     vim_free(fp);
 }
@@ -1117,7 +1111,7 @@ free_all_functions(void)
        for (hi = func_hashtab.ht_array; ; ++hi)
            if (!HASHITEM_EMPTY(hi))
            {
-               func_free(HI2UF(hi));
+               func_free(HI2UF(hi), TRUE);
                break;
            }
     hash_clear(&func_hashtab);
@@ -1331,7 +1325,7 @@ call_func(
                    if (--fp->uf_calls <= 0 && fp->uf_refcount <= 0)
                        /* Function was unreferenced while being used, free it
                         * now. */
-                       func_free(fp);
+                       func_free(fp, FALSE);
                    if (did_save_redo)
                        restoreRedobuff();
                    restore_search_patterns();
@@ -1674,6 +1668,20 @@ theend:
     return name;
 }
 
+/*
+ * 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"
  */
@@ -1721,7 +1729,7 @@ ex_function(exarg_T *eap)
                {
                    --todo;
                    fp = HI2UF(hi);
-                   if (!isdigit(*fp->uf_name))
+                   if (!func_name_refcount(fp->uf_name))
                        list_func_head(fp, FALSE);
                }
            }
@@ -2655,20 +2663,6 @@ get_user_func_name(expand_T *xp, int idx)
 
 #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}"
  */
@@ -2738,7 +2732,7 @@ ex_delfunction(exarg_T *eap)
                fp->uf_flags |= FC_DELETED;
            }
            else
-               func_free(fp);
+               func_free(fp, FALSE);
        }
     }
 }
@@ -2767,7 +2761,7 @@ func_unref(char_u *name)
        /* 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);
+           func_free(fp, FALSE);
     }
 }
 
@@ -2783,7 +2777,7 @@ func_ptr_unref(ufunc_T *fp)
        /* 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);
+           func_free(fp, FALSE);
     }
 }
 
@@ -3676,6 +3670,21 @@ set_ref_in_previous_funccal(int copyID)
     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,10 +3695,31 @@ set_ref_in_call_stack(int copyID)
     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)
     {
-       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);
+       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,9 +3741,6 @@ set_ref_in_func_args(int copyID)
 
 /*
  * 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
@@ -3725,6 +3752,7 @@ set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID)
     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,17 +3765,10 @@ set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID)
     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);
-           }
-       }
+           abort = abort || set_ref_in_funccal(fc, copyID);
     }
     vim_free(tofree);
-    return FALSE;
+    return abort;
 }
 
 #endif /* FEAT_EVAL */

diff --git a/src/version.c b/src/version.c
index 236166680fd122a5768f43e0fc2095e54b741fa2..1c7d2c388507be416baa727914b0f1ac4f7be561 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -763,6 +763,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2143,
 /**/
     2142,
 /**/