To: vim_dev@googlegroups.com Subject: Patch 8.0.0297 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.0.0297 Problem: Double free on exit when using a closure. (James McCoy) Solution: Split free_al_functions in two parts. (closes #1428) Files: src/userfunc.c, src/structs.h *** ../vim-8.0.0296/src/userfunc.c 2017-02-01 20:26:45.769640496 +0100 --- src/userfunc.c 2017-02-02 22:56:09.536572550 +0100 *************** *** 1075,1086 **** } /* ! * 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)); ga_clear_strings(&(fp->uf_lines)); --- 1075,1091 ---- } /* ! * 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_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,1105 **** 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, force); - vim_free(fp); } /* * There are two kinds of function names: * 1. ordinary names, function defined with :function * 2. numbered functions and lambdas --- 1094,1129 ---- 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); 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 * 2. numbered functions and lambdas *************** *** 1120,1129 **** hashitem_T *hi; ufunc_T *fp; long_u skipped = 0; ! long_u todo; ! /* Need to start all over every time, because func_free() may change the ! * hash table. */ while (func_hashtab.ht_used > skipped) { todo = func_hashtab.ht_used; --- 1144,1183 ---- hashitem_T *hi; ufunc_T *fp; long_u skipped = 0; ! 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; ! } ! } ! /* 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,1144 **** ++skipped; else { ! func_free(fp, TRUE); skipped = 0; break; } --- 1192,1198 ---- ++skipped; else { ! func_free(fp); skipped = 0; break; } *************** *** 1356,1362 **** 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(); --- 1410,1416 ---- if (--fp->uf_calls <= 0 && fp->uf_refcount <= 0) /* Function was unreferenced while being used, free it * now. */ ! func_clear_free(fp, FALSE); if (did_save_redo) restoreRedobuff(); restore_search_patterns(); *************** *** 2756,2762 **** fp->uf_flags |= FC_DELETED; } else ! func_free(fp, FALSE); } } } --- 2810,2816 ---- fp->uf_flags |= FC_DELETED; } else ! func_clear_free(fp, FALSE); } } } *************** *** 2785,2791 **** /* 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); } } --- 2839,2845 ---- /* Only delete it when it's not being used. Otherwise it's done * when "uf_calls" becomes zero. */ if (fp->uf_calls == 0) ! func_clear_free(fp, FALSE); } } *************** *** 2801,2807 **** /* 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); } } --- 2855,2861 ---- /* Only delete it when it's not being used. Otherwise it's done * when "uf_calls" becomes zero. */ if (fp->uf_calls == 0) ! func_clear_free(fp, FALSE); } } *** ../vim-8.0.0296/src/structs.h 2017-01-22 18:34:53.684030783 +0100 --- src/structs.h 2017-02-02 22:44:38.069397093 +0100 *************** *** 1337,1342 **** --- 1337,1343 ---- 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 *** ../vim-8.0.0296/src/version.c 2017-02-02 22:20:49.279397163 +0100 --- src/version.c 2017-02-02 22:44:52.229298214 +0100 *************** *** 766,767 **** --- 766,769 ---- { /* Add new patch number below this line */ + /**/ + 297, /**/ -- There is no right or wrong, there is only your personal opinion. (Bram Moolenaar) /// Bram Moolenaar -- Bram@Moolenaar.net -- 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 ///