patch 8.0.0297: double free on exit when using a closure

Problem:    Double free on exit when using a closure. (James McCoy)
Solution:   Split free_al_functions in two parts. (closes #1428)

diff --git a/src/structs.h b/src/structs.h
index af0a6fd2b7ef2d3d9a433fbc5bf6d60bdb44c5c9..45bd4a5edff8d7ae9a7e12fdb75611bc05313835 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1337,6 +1337,7 @@ typedef struct
     int                uf_varargs;     /* variable nr of arguments */
     int                uf_flags;
     int                uf_calls;       /* nr of active calls */
+    int                uf_cleared;     /* func_clear() was already called */
     garray_T   uf_args;        /* arguments */
     garray_T   uf_lines;       /* function lines */
 #ifdef FEAT_PROFILE

diff --git a/src/userfunc.c b/src/userfunc.c
index 516ab47078729fd2d2846840e09d86086a87b833..1bf028fa4241518741273b3d3c9c0acaf34e0375 100644 (file)
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -1075,12 +1075,17 @@ func_remove(ufunc_T *fp)
 }
 
 /*
- * Free a function and remove it from the list of functions.
+ * Free all things that a function contains.  Does not free the function
+ * itself, use func_free() for that.
  * When "force" is TRUE we are exiting.
  */
     static void
-func_free(ufunc_T *fp, int force)
+func_clear(ufunc_T *fp, int force)
 {
+    if (fp->uf_cleared)
+       return;
+    fp->uf_cleared = TRUE;
+
     /* clear this function */
     ga_clear_strings(&(fp->uf_args));
     ga_clear_strings(&(fp->uf_lines));
@@ -1089,16 +1094,35 @@ func_free(ufunc_T *fp, int force)
     vim_free(fp->uf_tml_total);
     vim_free(fp->uf_tml_self);
 #endif
+    funccal_unref(fp->uf_scoped, fp, force);
+}
+
+/*
+ * Free a function and remove it from the list of functions.  Does not free
+ * what a function contains, call func_clear() first.
+ */
+    static void
+func_free(ufunc_T *fp)
+{
     /* 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, force);
-
     vim_free(fp);
 }
 
+/*
+ * Free all things that a function contains and free the function itself.
+ * When "force" is TRUE we are exiting.
+ */
+    static void
+func_clear_free(ufunc_T *fp, int force)
+{
+    func_clear(fp, force);
+    func_free(fp);
+}
+
 /*
  * There are two kinds of function names:
  * 1. ordinary names, function defined with :function
@@ -1120,10 +1144,40 @@ free_all_functions(void)
     hashitem_T *hi;
     ufunc_T    *fp;
     long_u     skipped = 0;
-    long_u     todo;
+    long_u     todo = 1;
+    long_u     used;
+
+    /* First clear what the functions contain.  Since this may lower the
+     * reference count of a function, it may also free a function and change
+     * the hash table. Restart if that happens. */
+    while (todo > 0)
+    {
+       todo = func_hashtab.ht_used;
+       for (hi = func_hashtab.ht_array; todo > 0; ++hi)
+           if (!HASHITEM_EMPTY(hi))
+           {
+               /* Only free functions that are not refcounted, those are
+                * supposed to be freed when no longer referenced. */
+               fp = HI2UF(hi);
+               if (func_name_refcount(fp->uf_name))
+                   ++skipped;
+               else
+               {
+                   used = func_hashtab.ht_used;
+                   func_clear(fp, TRUE);
+                   if (used != func_hashtab.ht_used)
+                   {
+                       skipped = 0;
+                       break;
+                   }
+               }
+               --todo;
+           }
+    }
 
-    /* Need to start all over every time, because func_free() may change the
-     * hash table. */
+    /* Now actually free the functions.  Need to start all over every time,
+     * because func_free() may change the hash table. */
+    skipped = 0;
     while (func_hashtab.ht_used > skipped)
     {
        todo = func_hashtab.ht_used;
@@ -1138,7 +1192,7 @@ free_all_functions(void)
                    ++skipped;
                else
                {
-                   func_free(fp, TRUE);
+                   func_free(fp);
                    skipped = 0;
                    break;
                }
@@ -1356,7 +1410,7 @@ call_func(
                    if (--fp->uf_calls <= 0 && fp->uf_refcount <= 0)
                        /* Function was unreferenced while being used, free it
                         * now. */
-                       func_free(fp, FALSE);
+                       func_clear_free(fp, FALSE);
                    if (did_save_redo)
                        restoreRedobuff();
                    restore_search_patterns();
@@ -2756,7 +2810,7 @@ ex_delfunction(exarg_T *eap)
                fp->uf_flags |= FC_DELETED;
            }
            else
-               func_free(fp, FALSE);
+               func_clear_free(fp, FALSE);
        }
     }
 }
@@ -2785,7 +2839,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, FALSE);
+           func_clear_free(fp, FALSE);
     }
 }
 
@@ -2801,7 +2855,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, FALSE);
+           func_clear_free(fp, FALSE);
     }
 }
 

diff --git a/src/version.c b/src/version.c
index d7d3948e6863284dfb4b14b0a680aab8f8e65dbb..c85a57f6b83831b3b5b208d66f068370e745a6b2 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -764,6 +764,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    297,
 /**/
     296,
 /**/