6 files changed, 162 insertions, 114 deletions

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 0acdff3232..b004cfc7a1 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -657,7 +657,7 @@ bool object_to_vim(Object obj, typval_T *tv, Error *err)
         if (!object_to_vim(item, &li->li_tv, err)) {
           // cleanup
           listitem_free(li);
-          list_free(list, true);
+          list_free(list);
           return false;
         }
 
@@ -681,7 +681,7 @@ bool object_to_vim(Object obj, typval_T *tv, Error *err)
           api_set_error(err, Validation,
                         _("Empty dictionary keys aren't allowed"));
           // cleanup
-          dict_free(dict, true);
+          dict_free(dict);
           return false;
         }
 
@@ -690,7 +690,7 @@ bool object_to_vim(Object obj, typval_T *tv, Error *err)
         if (!object_to_vim(item.value, &di->di_tv, err)) {
           // cleanup
           dictitem_free(di);
-          dict_free(dict, true);
+          dict_free(dict);
           return false;
         }
 
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 8d5578fb50..2888ddc82d 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -4894,21 +4894,15 @@ static int get_lit_string_tv(char_u **arg, typval_T *rettv, int evaluate)
   return OK;
 }
 
-static void partial_free(partial_T *pt, bool recursive)
+static void partial_free(partial_T *pt)
 {
   int i;
 
   for (i = 0; i < pt->pt_argc; i++) {
-    typval_T *tv = &pt->pt_argv[i];
-
-    if (recursive || (tv->v_type != VAR_DICT && tv->v_type != VAR_LIST)) {
-      clear_tv(&pt->pt_argv[i]);
-    }
+    clear_tv(&pt->pt_argv[i]);
   }
   xfree(pt->pt_argv);
-  if (recursive) {
-    dict_unref(pt->pt_dict);
-  }
+  dict_unref(pt->pt_dict);
   func_unref(pt->pt_name);
   xfree(pt->pt_name);
   xfree(pt);
@@ -4919,22 +4913,7 @@ static void partial_free(partial_T *pt, bool recursive)
 void partial_unref(partial_T *pt)
 {
   if (pt != NULL && --pt->pt_refcount <= 0) {
-    partial_free(pt, true);
-  }
-}
-
-/// Like clear_tv(), but do not free lists or dictionaries.
-/// This is when called via free_unref_items().
-static void clear_tv_no_recurse(typval_T *tv) {
-  if (tv->v_type == VAR_PARTIAL) {
-    partial_T *pt = tv->vval.v_partial;
-
-    // We unref the partial but not the dict or any list it refers to
-    if (pt != NULL && --pt->pt_refcount == 0) {
-      partial_free(pt, false);
-    }
-  } else if (tv->v_type != VAR_LIST && tv->v_type != VAR_DICT) {
-    clear_tv(tv);
+    partial_free(pt);
   }
 }
 
@@ -4973,8 +4952,9 @@ static int get_list_tv(char_u **arg, typval_T *rettv, int evaluate)
   if (**arg != ']') {
     EMSG2(_("E697: Missing end of List ']': %s"), *arg);
 failret:
-    if (evaluate)
-      list_free(l, TRUE);
+    if (evaluate) {
+      list_free(l);
+    }
     return FAIL;
   }
 
@@ -5018,49 +4998,48 @@ static list_T *rettv_list_alloc(typval_T *rettv)
   return l;
 }
 
-/*
- * Unreference a list: decrement the reference count and free it when it
- * becomes zero.
- */
-void list_unref(list_T *l)
-{
-  if (l != NULL && --l->lv_refcount <= 0)
-    list_free(l, TRUE);
+/// Unreference a list: decrement the reference count and free it when it
+/// becomes zero.
+void list_unref(list_T *l) {
+  if (l != NULL && --l->lv_refcount <= 0) {
+    list_free(l);
+  }
 }
 
-/*
- * Free a list, including all items it points to.
- * Ignores the reference count.
- */
-void 
-list_free (
-    list_T *l,
-    int recurse            /* Free Lists and Dictionaries recursively. */
-)
-{
+/// Free a list, including all items it points to.
+/// Ignores the reference count.
+static void list_free_contents(list_T *l) {
   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;
-  else
-    l->lv_used_prev->lv_used_next = l->lv_used_next;
-  if (l->lv_used_next != NULL)
-    l->lv_used_next->lv_used_prev = l->lv_used_prev;
-
   for (item = l->lv_first; item != NULL; item = l->lv_first) {
-    /* Remove the item before deleting it. */
+    // Remove the item before deleting it.
     l->lv_first = item->li_next;
-    if (recurse) {
-      clear_tv(&item->li_tv);
-    } else {
-      clear_tv_no_recurse(&item->li_tv);
-    }
+    clear_tv(&item->li_tv);
     xfree(item);
   }
+}
+
+static void list_free_list(list_T *l) {
+  // Remove the list from the list of lists for garbage collection.
+  if (l->lv_used_prev == NULL) {
+    first_list = l->lv_used_next;
+  } else {
+    l->lv_used_prev->lv_used_next = l->lv_used_next;
+  }
+  if (l->lv_used_next != NULL) {
+    l->lv_used_next->lv_used_prev = l->lv_used_prev;
+  }
+
   xfree(l);
 }
 
+void list_free(list_T *l) {
+  if (!in_free_unref_items) {
+    list_free_contents(l);
+    list_free_list(l);
+  }
+}
+
 /*
  * Allocate a list item.
  * It is not initialized, don't forget to set v_lock.
@@ -5936,42 +5915,63 @@ bool garbage_collect(void)
 /// @return true, if something was freed.
 static int free_unref_items(int copyID)
 {
+  dict_T *dd, *dd_next;
+  list_T *ll, *ll_next;
   bool did_free = false;
 
+  // Let all "free" functions know that we are here. This means no
+  // dictionaries, lists, or jobs are to be freed, because we will
+  // do that here.
+  in_free_unref_items = true;
+
+  // PASS 1: free the contents of the items. We don't free the items
+  // themselves yet, so that it is possible to decrement refcount counters.
+
   // Go through the list of dicts and free items without the copyID.
   // Don't free dicts that are referenced internally.
-  for (dict_T *dd = first_dict; dd != NULL; ) {
+  for (dict_T *dd = first_dict; dd != NULL; dd = dd->dv_used_next) {
     if ((dd->dv_copyID & COPYID_MASK) != (copyID & COPYID_MASK)) {
       // 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_T *dd_next = dd->dv_used_next;
-      dict_free(dd, FALSE);
+      // of dicts or list of lists.
+      dict_free_contents(dd);
       did_free = true;
-      dd = dd_next;
-    } else {
-      dd = dd->dv_used_next;
     }
   }
 
   // Go through the list of lists and free items without the copyID.
   // But don't free a list that has a watcher (used in a for loop), these
   // are not referenced anywhere.
-  for (list_T *ll = first_list; ll != NULL;) {
+  for (list_T *ll = first_list; ll != NULL; ll = ll->lv_used_next) {
     if ((ll->lv_copyID & COPYID_MASK) != (copyID & COPYID_MASK)
         && 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_T* ll_next = ll->lv_used_next;
-      list_free(ll, FALSE);
+      list_free_contents(ll);
       did_free = true;
-      ll = ll_next;
-    } else {
-      ll = ll->lv_used_next;
     }
   }
 
+  // PASS 2: free the items themselves.
+  for (dd = first_dict; dd != NULL; dd = dd_next) {
+    dd_next = dd->dv_used_next;
+    if ((dd->dv_copyID & COPYID_MASK) != (copyID & COPYID_MASK)) {
+      dict_free_dict(dd);
+    }
+  }
+
+  for (ll = first_list; ll != NULL; ll = ll_next) {
+    ll_next = ll->lv_used_next;
+    if ((ll->lv_copyID & COPYID_MASK) != (copyID & COPYID_MASK)
+        && 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_list(ll);
+    }
+  }
+  in_free_unref_items = false;
   return did_free;
 }
 
@@ -6070,18 +6070,10 @@ bool set_ref_in_item(typval_T *tv, int copyID, ht_stack_T **ht_stack,
   FUNC_ATTR_WARN_UNUSED_RESULT
 {
   bool abort = false;
-  dict_T *dd;
 
   switch (tv->v_type) {
-    case VAR_PARTIAL:
     case VAR_DICT: {
-      if (tv->v_type == VAR_DICT) {
-        dd = tv->vval.v_dict;
-      } else if (tv->vval.v_partial != NULL) {
-        dd = tv->vval.v_partial->pt_dict;
-      } else {
-        dd = NULL;
-      }
+      dict_T *dd = tv->vval.v_dict;
 
       if (dd != NULL && dd->dv_copyID != copyID) {
         // Didn't see this dict yet.
@@ -6134,6 +6126,27 @@ bool set_ref_in_item(typval_T *tv, int copyID, ht_stack_T **ht_stack,
       break;
     }
 
+    case VAR_PARTIAL: {
+      partial_T *pt = tv->vval.v_partial;
+      int i;
+
+      // A partial does not have a copyID, because it cannot contain itself.
+      if (pt != NULL) {
+        if (pt->pt_dict != NULL) {
+          typval_T dtv;
+
+          dtv.v_type = VAR_DICT;
+          dtv.vval.v_dict = pt->pt_dict;
+          abort = abort || set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+        }
+
+        for (i = 0; i < pt->pt_argc; i++) {
+          abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID,
+                                           ht_stack, list_stack);
+        }
+      }
+      break;
+    }
     case VAR_FUNC:
     case VAR_UNKNOWN:
     case VAR_SPECIAL:
@@ -6257,31 +6270,19 @@ void dict_clear(dict_T *d)
  */
 void dict_unref(dict_T *d)
 {
-  if (d != NULL && --d->dv_refcount <= 0)
-    dict_free(d, TRUE);
+  if (d != NULL && --d->dv_refcount <= 0) {
+    dict_free(d);
+  }
 }
 
-/*
- * Free a Dictionary, including all items it contains.
- * Ignores the reference count.
- */
-void 
-dict_free (
-    dict_T *d,
-    int recurse            /* Free Lists and Dictionaries recursively. */
-)
-{
+/// Free a Dictionary, including all items it contains.
+/// Ignores the reference count.
+static void dict_free_contents(dict_T *d) {
   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;
-  else
-    d->dv_used_prev->dv_used_next = d->dv_used_next;
-  if (d->dv_used_next != NULL)
-    d->dv_used_next->dv_used_prev = d->dv_used_prev;
+
 
   /* Lock the hashtab, we don't want it to resize while freeing items. */
   hash_lock(&d->dv_hashtab);
@@ -6293,11 +6294,7 @@ dict_free (
        * something recursive causing trouble. */
       di = HI2DI(hi);
       hash_remove(&d->dv_hashtab, hi);
-      if (recurse) {
-        clear_tv(&di->di_tv);
-      } else {
-        clear_tv_no_recurse(&di->di_tv);
-      }
+      clear_tv(&di->di_tv);
       xfree(di);
       --todo;
     }
@@ -6311,9 +6308,29 @@ dict_free (
   }
 
   hash_clear(&d->dv_hashtab);
+}
+
+static void dict_free_dict(dict_T *d) {
+  // Remove the dict from the list of dicts for garbage collection.
+  if (d->dv_used_prev == NULL) {
+    first_dict = d->dv_used_next;
+  } else {
+    d->dv_used_prev->dv_used_next = d->dv_used_next;
+  }
+  if (d->dv_used_next != NULL) {
+    d->dv_used_next->dv_used_prev = d->dv_used_prev;
+  }
+
   xfree(d);
 }
 
+void dict_free(dict_T *d) {
+  if (!in_free_unref_items) {
+    dict_free_contents(d);
+    dict_free_dict(d);
+  }
+}
+
 /*
  * Allocate a Dictionary item.
  * The "key" is copied to the new item.
@@ -6720,8 +6737,9 @@ static int get_dict_tv(char_u **arg, typval_T *rettv, int evaluate)
   if (**arg != '}') {
     EMSG2(_("E723: Missing end of Dictionary '}': %s"), *arg);
 failret:
-    if (evaluate)
-      dict_free(d, TRUE);
+    if (evaluate) {
+      dict_free(d);
+    }
     return FAIL;
   }
 
@@ -18547,7 +18565,7 @@ void free_tv(typval_T *varp)
       tv->v_lock = VAR_UNLOCKED; \
     } while (0)
 
-#define TYPVAL_ENCODE_CONV_PARTIAL(partial) \
+#define TYPVAL_ENCODE_CONV_PARTIAL(pt) \
     do { \
       tv->v_lock = VAR_UNLOCKED; \
     } while (0)
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index acdda9b657..85ec305778 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -1235,6 +1235,8 @@ EXTERN FILE *time_fd INIT(= NULL);  /* where to write startup timing */
 EXTERN int ignored;
 EXTERN char *ignoredp;
 
+EXTERN int in_free_unref_items INIT(= false);
+
 // If a msgpack-rpc channel should be started over stdin/stdout
 EXTERN bool embedded_mode INIT(= false);
 
diff --git a/src/nvim/tag.c b/src/nvim/tag.c
index 1df1952f53..597f737410 100644
--- a/src/nvim/tag.c
+++ b/src/nvim/tag.c
@@ -790,7 +790,7 @@ do_tag (
         vim_snprintf((char *)IObuff, IOSIZE, "ltag %s", tag);
         set_errorlist(curwin, list, ' ', IObuff);
 
-        list_free(list, TRUE);
+        list_free(list);
         xfree(fname);
         xfree(cmd);
 
diff --git a/src/nvim/testdir/test_partial.vim b/src/nvim/testdir/test_partial.vim
index b5909910c7..7562c7fd2a 100644
--- a/src/nvim/testdir/test_partial.vim
+++ b/src/nvim/testdir/test_partial.vim
@@ -193,7 +193,7 @@ func Test_redefine_dict_func()
   endtry
 endfunc
 
-" This causes double free on exit if EXITFREE is defined.
+" This caused double free on exit if EXITFREE is defined.
 func Test_cyclic_list_arg()
   let l = []
   let Pt = function('string', [l])
@@ -202,7 +202,7 @@ func Test_cyclic_list_arg()
   unlet Pt
 endfunc
 
-" This causes double free on exit if EXITFREE is defined.
+" This caused double free on exit if EXITFREE is defined.
 func Test_cyclic_dict_arg()
   let d = {}
   let Pt = function('string', [d])
@@ -211,6 +211,21 @@ func Test_cyclic_dict_arg()
   unlet Pt
 endfunc
 
+func Ignored(job1, job2, status)
+endfunc
+
+" func Test_cycle_partial_job()
+"   let job = job_start('echo')
+"   call job_setoptions(job, {'exit_cb': function('Ignored', [job])})
+"   unlet job
+" endfunc
+
+" func Test_ref_job_partial_dict()
+"   let g:ref_job = job_start('echo')
+"   let d = {'a': 'b'}
+"   call job_setoptions(g:ref_job, {'exit_cb': function('string', [], d)})
+" endfunc
+
 func Test_auto_partial_rebind()
   let dict1 = {'name': 'dict1'}
   func! dict1.f1()
diff --git a/src/nvim/testdir/test_timers.vim b/src/nvim/testdir/test_timers.vim
index 9f58a35909..abf4e43ec3 100644
--- a/src/nvim/testdir/test_timers.vim
+++ b/src/nvim/testdir/test_timers.vim
@@ -30,3 +30,16 @@ func Test_repeat_many()
   call assert_true(s:val > 1)
   call assert_true(s:val < 5)
 endfunc
+
+" func Test_with_partial_callback()
+"   let s:val = 0
+"   let s:meow = {}
+"   function s:meow.bite(...)
+"     let s:val += 1
+"   endfunction
+
+"   call timer_start(50, s:meow.bite)
+"   sleep 200m
+"   call assert_equal(1, s:val)
+" endfunc
+" vim: ts=2 sw=0 et