patch 8.2.3809: Vim9: crash when garbage collecting a nested partialv8.2.3809

Problem:    Vim9: crash when garbage collecting a nested partial. (Virginia
            Senioria)
Solution:   Set references in all the funcstacks. (closes #9348)

6 files changed, 100 insertions, 12 deletions

diff --git a/src/eval.c b/src/eval.c
index abfde87f3..0a85b909d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -4510,6 +4510,9 @@ garbage_collect(int testing)
     // function call arguments, if v:testing is set.
     abort = abort || set_ref_in_func_args(copyID);
 
+    // funcstacks keep variables for closures
+    abort = abort || set_ref_in_funcstacks(copyID);
+
     // v: vars
     abort = abort || garbage_collect_vimvars(copyID);
 
@@ -4869,15 +4872,7 @@ set_ref_in_item(
 	    for (i = 0; i < pt->pt_argc; ++i)
 		abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID,
 							ht_stack, list_stack);
-	    if (pt->pt_funcstack != NULL)
-	    {
-		typval_T    *stack = pt->pt_funcstack->fs_ga.ga_data;
-
-		for (i = 0; i < pt->pt_funcstack->fs_ga.ga_len; ++i)
-		    abort = abort || set_ref_in_item(stack + i, copyID,
-							 ht_stack, list_stack);
-	    }
-
+	    // pt_funcstack is handled in set_ref_in_funcstacks()
 	}
     }
 #ifdef FEAT_JOB_CHANNEL
diff --git a/src/proto/vim9execute.pro b/src/proto/vim9execute.pro
index ef22af75b..6f42fc865 100644
--- a/src/proto/vim9execute.pro
+++ b/src/proto/vim9execute.pro
@@ -1,6 +1,7 @@
 /* vim9execute.c */
 void to_string_error(vartype_T vartype);
 void funcstack_check_refcount(funcstack_T *funcstack);
+int set_ref_in_funcstacks(int copyID);
 char_u *char_from_string(char_u *str, varnumber_T index);
 char_u *string_slice(char_u *str, varnumber_T first, varnumber_T last, int exclusive);
 int fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx);
diff --git a/src/structs.h b/src/structs.h
index 5b7739ed5..9aad4152e 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -2009,8 +2009,13 @@ typedef struct {
  * Structure to hold the context of a compiled function, used by closures
  * defined in that function.
  */
-typedef struct funcstack_S
+typedef struct funcstack_S funcstack_T;
+
+struct funcstack_S
 {
+    funcstack_T *fs_next;	// linked list at "first_funcstack"
+    funcstack_T *fs_prev;
+
     garray_T	fs_ga;		// contains the stack, with:
 				// - arguments
 				// - frame
@@ -2021,7 +2026,7 @@ typedef struct funcstack_S
     int		fs_refcount;	// nr of closures referencing this funcstack
     int		fs_min_refcount; // nr of closures on this funcstack
     int		fs_copyID;	// for garray_T collection
-} funcstack_T;
+};
 
 typedef struct outer_S outer_T;
 struct outer_S {
diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index 8ffaf976f..f4a42cec2 100644
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -1903,6 +1903,41 @@ def Test_delfunc()
   delete('XToDelFunc')
 enddef
 
+func Test_free_dict_while_in_funcstack()
+  " relies on the sleep command
+  CheckUnix
+  call Run_Test_free_dict_while_in_funcstack()
+endfunc
+
+def Run_Test_free_dict_while_in_funcstack()
+
+  # this was freeing the TermRun() default argument dictionary while it was
+  # still referenced in a funcstack_T
+  var lines =<< trim END
+      vim9script
+
+      &updatetime = 400
+      def TermRun(_ = {})
+          def Post()
+          enddef
+          def Exec()
+              term_start('sleep 1', {
+                  term_finish: 'close',
+                  exit_cb: (_, _) => Post(),
+              })
+          enddef
+          Exec()
+      enddef
+      nnoremap <F4> <Cmd>call <SID>TermRun()<CR>
+      timer_start(100, (_) => feedkeys("\<F4>"))
+      timer_start(1000, (_) => feedkeys("\<F4>"))
+      sleep 1500m
+  END
+  CheckScriptSuccess(lines)
+  nunmap <F4>
+  set updatetime&
+enddef
+
 def Test_redef_failure()
   writefile(['def Func0(): string',  'return "Func0"', 'enddef'], 'Xdef')
   so Xdef
diff --git a/src/version.c b/src/version.c
index 6410f4fa8..c7f6fe796 100644
--- a/src/version.c
+++ b/src/version.c
@@ -750,6 +750,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    3809,
+/**/
     3808,
 /**/
     3807,
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 3160f0df2..53beee103 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -468,6 +468,31 @@ call_dfunc(
 // Get pointer to item in the stack.
 #define STACK_TV(idx) (((typval_T *)ectx->ec_stack.ga_data) + idx)
 
+// Double linked list of funcstack_T in use.
+static funcstack_T *first_funcstack = NULL;
+
+    static void
+add_funcstack_to_list(funcstack_T *funcstack)
+{
+	// Link in list of funcstacks.
+    if (first_funcstack != NULL)
+	first_funcstack->fs_prev = funcstack;
+    funcstack->fs_next = first_funcstack;
+    funcstack->fs_prev = NULL;
+    first_funcstack = funcstack;
+}
+
+    static void
+remove_funcstack_from_list(funcstack_T *funcstack)
+{
+    if (funcstack->fs_prev == NULL)
+	first_funcstack = funcstack->fs_next;
+    else
+	funcstack->fs_prev->fs_next = funcstack->fs_next;
+    if (funcstack->fs_next != NULL)
+	funcstack->fs_next->fs_prev = funcstack->fs_prev;
+}
+
 /*
  * Used when returning from a function: Check if any closure is still
  * referenced.  If so then move the arguments and variables to a separate piece
@@ -540,6 +565,7 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
 	// Move them to the called function.
 	if (funcstack == NULL)
 	    return FAIL;
+
 	funcstack->fs_var_offset = argcount + STACK_FRAME_SIZE;
 	funcstack->fs_ga.ga_len = funcstack->fs_var_offset + dfunc->df_varcount;
 	stack = ALLOC_CLEAR_MULT(typval_T, funcstack->fs_ga.ga_len);
@@ -549,6 +575,7 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
 	    vim_free(funcstack);
 	    return FAIL;
 	}
+	add_funcstack_to_list(funcstack);
 
 	// Move or copy the arguments.
 	for (idx = 0; idx < argcount; ++idx)
@@ -571,7 +598,7 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
 	    // local variable, has a reference count for the variable, thus
 	    // will never go down to zero.  When all these refcounts are one
 	    // then the funcstack is unused.  We need to count how many we have
-	    // so we need when to check.
+	    // so we know when to check.
 	    if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL)
 	    {
 		int	    i;
@@ -643,11 +670,34 @@ funcstack_check_refcount(funcstack_T *funcstack)
 	for (i = 0; i < gap->ga_len; ++i)
 	    clear_tv(stack + i);
 	vim_free(stack);
+	remove_funcstack_from_list(funcstack);
 	vim_free(funcstack);
     }
 }
 
 /*
+ * For garbage collecting: set references in all variables referenced by
+ * all funcstacks.
+ */
+    int
+set_ref_in_funcstacks(int copyID)
+{
+    funcstack_T *funcstack;
+
+    for (funcstack = first_funcstack; funcstack != NULL;
+						funcstack = funcstack->fs_next)
+    {
+	typval_T    *stack = funcstack->fs_ga.ga_data;
+	int	    i;
+
+	for (i = 0; i < funcstack->fs_ga.ga_len; ++i)
+	    if (set_ref_in_item(stack + i, copyID, NULL, NULL))
+		return TRUE;  // abort
+    }
+    return FALSE;
+}
+
+/*
  * Return from the current function.
  */
     static int