To: vim-dev@vim.org

Subject: Patch 7.0.135

Fcc: outbox

From: Bram Moolenaar <Bram@moolenaar.net>

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    ///