To: vim-dev@vim.org Subject: Patch 7.0.135 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit ------------ Patch 7.0.135 Problem: Crash when garbage collecting list or dict with loop. Solution: Don't use DEL_REFCOUNT but don't recurse into Lists and Dictionaries when freeing them in the garbage collector. Also add allocated Dictionaries to the list of Dictionaries to avoid leaking memory. Files: src/eval.c, src/proto/eval.pro, src/tag.c *** ../vim-7.0.134/src/eval.c Sun Oct 15 15:10:08 2006 --- src/eval.c Sun Oct 15 22:30:09 2006 *************** *** 191,198 **** #define FC_RANGE 2 /* function accepts range */ #define FC_DICT 4 /* Dict function, uses "self" */ - #define DEL_REFCOUNT 999999 /* list/dict is being deleted */ - /* * All user-defined functions are found in this hashtable. */ --- 191,196 ---- *************** *** 435,441 **** static void set_ref_in_list __ARGS((list_T *l, int copyID)); static void set_ref_in_item __ARGS((typval_T *tv, int copyID)); static void dict_unref __ARGS((dict_T *d)); ! static void dict_free __ARGS((dict_T *d)); static dictitem_T *dictitem_alloc __ARGS((char_u *key)); static dictitem_T *dictitem_copy __ARGS((dictitem_T *org)); static void dictitem_remove __ARGS((dict_T *dict, dictitem_T *item)); --- 433,439 ---- static void set_ref_in_list __ARGS((list_T *l, int copyID)); static void set_ref_in_item __ARGS((typval_T *tv, int copyID)); static void dict_unref __ARGS((dict_T *d)); ! static void dict_free __ARGS((dict_T *d, int recurse)); static dictitem_T *dictitem_alloc __ARGS((char_u *key)); static dictitem_T *dictitem_copy __ARGS((dictitem_T *org)); static void dictitem_remove __ARGS((dict_T *dict, dictitem_T *item)); *************** *** 4899,4905 **** { if (list_append_tv(l, &item->li_tv) == FAIL) { ! list_free(l); return FAIL; } item = item->li_next; --- 4897,4903 ---- { if (list_append_tv(l, &item->li_tv) == FAIL) { ! list_free(l, TRUE); return FAIL; } item = item->li_next; *************** *** 5299,5305 **** EMSG2(_("E697: Missing end of List ']': %s"), *arg); failret: if (evaluate) ! list_free(l); return FAIL; } --- 5297,5303 ---- EMSG2(_("E697: Missing end of List ']': %s"), *arg); failret: if (evaluate) ! list_free(l, TRUE); return FAIL; } *************** *** 5363,5370 **** list_unref(l) list_T *l; { ! if (l != NULL && l->lv_refcount != DEL_REFCOUNT && --l->lv_refcount <= 0) ! list_free(l); } /* --- 5361,5368 ---- list_unref(l) list_T *l; { ! if (l != NULL && --l->lv_refcount <= 0) ! list_free(l, TRUE); } /* *************** *** 5372,5385 **** * Ignores the reference count. */ void ! list_free(l) ! list_T *l; { listitem_T *item; - /* Avoid that recursive reference to the list frees us again. */ - l->lv_refcount = DEL_REFCOUNT; - /* Remove the list from the list of lists for garbage collection. */ if (l->lv_used_prev == NULL) first_list = l->lv_used_next; --- 5370,5381 ---- * Ignores the reference count. */ void ! list_free(l, recurse) ! list_T *l; ! int recurse; /* Free Lists and Dictionaries recursively. */ { listitem_T *item; /* Remove the list from the list of lists for garbage collection. */ if (l->lv_used_prev == NULL) first_list = l->lv_used_next; *************** *** 5392,5398 **** { /* Remove the item before deleting it. */ l->lv_first = item->li_next; ! listitem_free(item); } vim_free(l); } --- 5388,5397 ---- { /* Remove the item before deleting it. */ l->lv_first = item->li_next; ! if (recurse || (item->li_tv.v_type != VAR_LIST ! && item->li_tv.v_type != VAR_DICT)) ! clear_tv(&item->li_tv); ! vim_free(item); } vim_free(l); } *************** *** 6113,6119 **** for (dd = first_dict; dd != NULL; ) if (dd->dv_copyID != copyID) { ! dict_free(dd); did_free = TRUE; /* restart, next dict may also have been freed */ --- 6118,6127 ---- for (dd = first_dict; dd != NULL; ) if (dd->dv_copyID != copyID) { ! /* Free the Dictionary and ordinary items it contains, but don't ! * recurse into Lists and Dictionaries, they will be in the list ! * of dicts or list of lists. */ ! dict_free(dd, FALSE); did_free = TRUE; /* restart, next dict may also have been freed */ *************** *** 6130,6136 **** for (ll = first_list; ll != NULL; ) if (ll->lv_copyID != copyID && ll->lv_watch == NULL) { ! list_free(ll); did_free = TRUE; /* restart, next list may also have been freed */ --- 6138,6147 ---- for (ll = first_list; ll != NULL; ) if (ll->lv_copyID != copyID && ll->lv_watch == NULL) { ! /* Free the List and ordinary items it contains, but don't recurse ! * into Lists and Dictionaries, they will be in the list of dicts ! * or list of lists. */ ! list_free(ll, FALSE); did_free = TRUE; /* restart, next list may also have been freed */ *************** *** 6223,6233 **** d = (dict_T *)alloc(sizeof(dict_T)); if (d != NULL) { ! /* Add the list to the hashtable for garbage collection. */ if (first_dict != NULL) first_dict->dv_used_prev = d; d->dv_used_next = first_dict; d->dv_used_prev = NULL; hash_init(&d->dv_hashtab); d->dv_lock = 0; --- 6234,6245 ---- d = (dict_T *)alloc(sizeof(dict_T)); if (d != NULL) { ! /* Add the list to the list of dicts for garbage collection. */ if (first_dict != NULL) first_dict->dv_used_prev = d; d->dv_used_next = first_dict; d->dv_used_prev = NULL; + first_dict = d; hash_init(&d->dv_hashtab); d->dv_lock = 0; *************** *** 6245,6252 **** dict_unref(d) dict_T *d; { ! if (d != NULL && d->dv_refcount != DEL_REFCOUNT && --d->dv_refcount <= 0) ! dict_free(d); } /* --- 6257,6264 ---- dict_unref(d) dict_T *d; { ! if (d != NULL && --d->dv_refcount <= 0) ! dict_free(d, TRUE); } /* *************** *** 6254,6269 **** * Ignores the reference count. */ static void ! dict_free(d) ! dict_T *d; { int todo; hashitem_T *hi; dictitem_T *di; - /* Avoid that recursive reference to the dict frees us again. */ - d->dv_refcount = DEL_REFCOUNT; - /* Remove the dict from the list of dicts for garbage collection. */ if (d->dv_used_prev == NULL) first_dict = d->dv_used_next; --- 6266,6279 ---- * Ignores the reference count. */ static void ! dict_free(d, recurse) ! dict_T *d; ! int recurse; /* Free Lists and Dictionaries recursively. */ { int todo; hashitem_T *hi; dictitem_T *di; /* Remove the dict from the list of dicts for garbage collection. */ if (d->dv_used_prev == NULL) first_dict = d->dv_used_next; *************** *** 6283,6289 **** * something recursive causing trouble. */ di = HI2DI(hi); hash_remove(&d->dv_hashtab, hi); ! dictitem_free(di); --todo; } } --- 6293,6302 ---- * something recursive causing trouble. */ di = HI2DI(hi); hash_remove(&d->dv_hashtab, hi); ! if (recurse || (di->di_tv.v_type != VAR_LIST ! && di->di_tv.v_type != VAR_DICT)) ! clear_tv(&di->di_tv); ! vim_free(di); --todo; } } *************** *** 6734,6740 **** EMSG2(_("E723: Missing end of Dictionary '}': %s"), *arg); failret: if (evaluate) ! dict_free(d); return FAIL; } --- 6747,6753 ---- EMSG2(_("E723: Missing end of Dictionary '}': %s"), *arg); failret: if (evaluate) ! dict_free(d, TRUE); return FAIL; } *** ../vim-7.0.134/src/proto/eval.pro Fri Mar 24 23:16:28 2006 --- src/proto/eval.pro Sun Oct 15 22:08:11 2006 *************** *** 44,50 **** extern char_u *get_user_var_name __ARGS((expand_T *xp, int idx)); extern list_T *list_alloc __ARGS((void)); extern void list_unref __ARGS((list_T *l)); ! extern void list_free __ARGS((list_T *l)); extern dictitem_T *dict_lookup __ARGS((hashitem_T *hi)); extern int list_append_dict __ARGS((list_T *list, dict_T *dict)); extern int garbage_collect __ARGS((void)); --- 44,50 ---- extern char_u *get_user_var_name __ARGS((expand_T *xp, int idx)); extern list_T *list_alloc __ARGS((void)); extern void list_unref __ARGS((list_T *l)); ! extern void list_free __ARGS((list_T *l, int recurse)); extern dictitem_T *dict_lookup __ARGS((hashitem_T *hi)); extern int list_append_dict __ARGS((list_T *list, dict_T *dict)); extern int garbage_collect __ARGS((void)); *** ../vim-7.0.134/src/tag.c Sun Sep 10 13:56:06 2006 --- src/tag.c Sun Oct 15 21:44:56 2006 *************** *** 911,917 **** set_errorlist(curwin, list, ' '); ! list_free(list); cur_match = 0; /* Jump to the first tag */ } --- 911,917 ---- set_errorlist(curwin, list, ' '); ! list_free(list, TRUE); cur_match = 0; /* Jump to the first tag */ } *** ../vim-7.0.134/src/version.c Sun Oct 15 15:10:08 2006 --- src/version.c Sun Oct 15 22:01:53 2006 *************** *** 668,669 **** --- 668,671 ---- { /* Add new patch number below this line */ + /**/ + 135, /**/ -- Well, you come from nothing, you go back to nothing... What have you lost? Nothing! -- Monty Python: The life of Brian /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ download, build and distribute -- http://www.A-A-P.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///