patch 8.2.3809: Vim9: crash when garbage collecting a nested partial

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

diff --git a/src/eval.c b/src/eval.c
index abfde87f358b5f15d8d196128323bb1019c8bfdc..0a85b909d0e1d6178dda453982ada545c89050c6 100644 (file)
--- 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 ef22af75b490b72b2c23798ea74199d1d1a4aced..6f42fc8655073064365ff297d3f4c931dfbe5f76 100644 (file)
--- 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 5b7739ed583098d7b43360c7226b59f7c396c1d5..9aad4152e498c9634dd6af9a83b220707f9f092a 100644 (file)
--- 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 8ffaf976f6332c7a4512c0f64e4c55675392f1db..f4a42cec259634dc1da55743ad10efbaa33a6553 100644 (file)
--- 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 6410f4fa803bcebdf4e60643d1db4118acde5dd9..c7f6fe796a8483c5bf5101df021eaf5de06e8f52 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -749,6 +749,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    3809,
 /**/
     3808,
 /**/

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 3160f0df2e1d325b41a494d77a37d1bbb09a1834..53beee10343fe7e5cac55663f5952e70bf642fd5 100644 (file)
--- 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,10 +670,33 @@ 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.
  */