patch 7.4.2136

Problem:    Closure function fails.
Solution:   Don't reset uf_scoped when it points to another funccal.

diff --git a/src/testdir/test_lambda.vim b/src/testdir/test_lambda.vim
index 511873807618e9eaa36177169214400d8a422265..4d895460e565563e824cc9d67474b47178dd6a7e 100644 (file)
--- a/src/testdir/test_lambda.vim
+++ b/src/testdir/test_lambda.vim
@@ -247,3 +247,27 @@ function! Test_closure_unlet()
   call assert_false(has_key(s:foo(), 'x'))
   call test_garbagecollect_now()
 endfunction
+
+function! LambdaFoo()
+  let x = 0
+  function! LambdaBar() closure
+    let x += 1
+    return x
+  endfunction
+  return function('LambdaBar')
+endfunction
+
+func Test_closure_refcount()
+  let g:Count = LambdaFoo()
+  call test_garbagecollect_now()
+  call assert_equal(1, g:Count())
+  let g:Count2 = LambdaFoo()
+  call test_garbagecollect_now()
+  call assert_equal(1, g:Count2())
+  call assert_equal(2, g:Count())
+  call assert_equal(3, g:Count2())
+
+  " This causes memory access errors.
+  " delfunc LambdaFoo
+  " delfunc LambdaBar
+endfunc

diff --git a/src/userfunc.c b/src/userfunc.c
index 3c6401f7d7b26b4ccd7000ff69fc7061cf6d2f4b..d264e5bbbb4355407b3dc33b5cd12f251dfbebb9 100644 (file)
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -150,6 +150,7 @@ static int
 # endif
        prof_self_cmp(const void *s1, const void *s2);
 #endif
+static void funccal_unref(funccall_T *fc, ufunc_T *fp);
 
     void
 func_init()
@@ -257,6 +258,23 @@ err_ret:
     return FAIL;
 }
 
+/*
+ * Register function "fp" as using "current_funccal" as its scope.
+ */
+    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(&current_funccal->fc_funcs, 1) == FAIL)
+       return FAIL;
+    ((ufunc_T **)current_funccal->fc_funcs.ga_data)
+       [current_funccal->fc_funcs.ga_len++] = fp;
+    func_ref(current_funccal->func->uf_name);
+    return OK;
+}
+
 /*
  * Parse a lambda expression and get a Funcref from "*arg".
  * Return OK or FAIL.  Returns NOTDONE for dict or {expr}.
@@ -318,7 +336,7 @@ get_lambda_tv(char_u **arg, typval_T *rettv, int evaluate)
 
        sprintf((char*)name, "<lambda>%d", ++lambda_no);
 
-       fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name)));
+       fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name)));
        if (fp == NULL)
            goto errret;
 
@@ -343,13 +361,8 @@ get_lambda_tv(char_u **arg, typval_T *rettv, int evaluate)
        if (current_funccal != NULL && eval_lavars)
        {
            flags |= FC_CLOSURE;
-           fp->uf_scoped = current_funccal;
-           current_funccal->fc_refcount++;
-           if (ga_grow(&current_funccal->fc_funcs, 1) == FAIL)
+           if (register_closure(fp) == FAIL)
                goto errret;
-           ((ufunc_T **)current_funccal->fc_funcs.ga_data)
-                                   [current_funccal->fc_funcs.ga_len++] = fp;
-           func_ref(current_funccal->func->uf_name);
        }
        else
            fp->uf_scoped = NULL;
@@ -660,8 +673,15 @@ free_funccal(
        ufunc_T     *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
 
        if (fp != NULL)
-           fp->uf_scoped = 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_unref(fc->func->uf_name);
+       }
     }
+    ga_clear(&fc->fc_funcs);
 
     /* The a: variables typevals may not have been allocated, only free the
      * allocated variables. */
@@ -675,15 +695,6 @@ free_funccal(
        for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)
            clear_tv(&li->li_tv);
 
-    for (i = 0; i < fc->fc_funcs.ga_len; ++i)
-    {
-       ufunc_T     *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
-
-       if (fp != NULL)
-           func_unref(fc->func->uf_name);
-    }
-    ga_clear(&fc->fc_funcs);
-
     func_unref(fc->func->uf_name);
     vim_free(fc);
 }
@@ -1046,8 +1057,8 @@ call_user_func(
     current_funccal = fc->caller;
     --depth;
 
-    /* If the a:000 list and the l: and a: dicts are not referenced we can
-     * free the funccall_T and what's in it. */
+    /* If the a:000 list and the l: and a: dicts are not referenced and there
+     * is no closure using it, we can free the funccall_T and what's in it. */
     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
@@ -1061,8 +1072,8 @@ call_user_func(
        listitem_T      *li;
        int             todo;
 
-       /* "fc" is still in use.  This can happen when returning "a:000" or
-        * assigning "l:" to a global variable.
+       /* "fc" is still in use.  This can happen when returning "a:000",
+        * assigning "l:" to a global variable or defining a closure.
         * Link "fc" in the list for garbage collection later. */
        fc->caller = previous_funccal;
        previous_funccal = fc;
@@ -1121,13 +1132,11 @@ funccal_unref(funccall_T *fc, ufunc_T *fp)
        func_unref(fc->func->uf_name);
 
        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;
            }
-       }
     }
 }
 
@@ -1976,6 +1985,12 @@ ex_function(exarg_T *eap)
        {
            flags |= FC_CLOSURE;
            p += 7;
+           if (current_funccal == NULL)
+           {
+               emsg_funcname(N_("E932 Closure function should not be at top level: %s"),
+                       name == NULL ? (char_u *)"" : name);
+               goto erret;
+           }
        }
        else
            break;
@@ -2265,7 +2280,7 @@ ex_function(exarg_T *eap)
            }
        }
 
-       fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name)));
+       fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name)));
        if (fp == NULL)
            goto erret;
 
@@ -2311,19 +2326,9 @@ ex_function(exarg_T *eap)
     fp->uf_lines = newlines;
     if ((flags & FC_CLOSURE) != 0)
     {
-       if (current_funccal == NULL)
-       {
-           emsg_funcname(N_("E932 Closure function should not be at top level: %s"),
-                   name);
+       ++fp->uf_refcount;
+       if (register_closure(fp) == FAIL)
            goto erret;
-       }
-       fp->uf_scoped = current_funccal;
-       current_funccal->fc_refcount++;
-       if (ga_grow(&current_funccal->fc_funcs, 1) == FAIL)
-           goto erret;
-       ((ufunc_T **)current_funccal->fc_funcs.ga_data)
-                               [current_funccal->fc_funcs.ga_len++] = fp;
-       func_ref(current_funccal->func->uf_name);
     }
     else
        fp->uf_scoped = NULL;
@@ -3582,21 +3587,21 @@ find_hi_in_scoped_ht(char_u *name, char_u **varname, hashtab_T **pht)
 
     /* Search in parent scope which is possible to reference from lambda */
     current_funccal = current_funccal->func->uf_scoped;
-    while (current_funccal)
+    while (current_funccal != NULL)
     {
-      ht = find_var_ht(name, varname);
-      if (ht != NULL && **varname != NUL)
-      {
-         hi = hash_find(ht, *varname);
-         if (!HASHITEM_EMPTY(hi))
-         {
-             *pht = ht;
-             break;
-         }
-      }
-      if (current_funccal == current_funccal->func->uf_scoped)
-         break;
-      current_funccal = current_funccal->func->uf_scoped;
+       ht = find_var_ht(name, varname);
+       if (ht != NULL && **varname != NUL)
+       {
+           hi = hash_find(ht, *varname);
+           if (!HASHITEM_EMPTY(hi))
+           {
+               *pht = ht;
+               break;
+           }
+       }
+       if (current_funccal == current_funccal->func->uf_scoped)
+           break;
+       current_funccal = current_funccal->func->uf_scoped;
     }
     current_funccal = old_current_funccal;
 

diff --git a/src/version.c b/src/version.c
index 210c2e4c055db0358b3e8e1e66acc23c05d7b51a..7e1b45aa887c0efaf3322ee1a45fac222ca54ffb 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 */
+/**/
+    2136,
 /**/
     2135,
 /**/