# HG changeset patch # User Bram Moolenaar # Date 1639505704 -3600 # Node ID 454a1c9ef7970a0144668bfc5d2cc3fc09b071cd # Parent 177872ca0db9434f76489b968cbf229d6e140b8d patch 8.2.3809: Vim9: crash when garbage collecting a nested partial Commit: https://github.com/vim/vim/commit/7509ad8b0fad56f88288977decbeca3640406c82 Author: Bram Moolenaar Date: Tue Dec 14 18:14:37 2021 +0000 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 --- 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 --- 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 --- 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 --- 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 call TermRun() + timer_start(100, (_) => feedkeys("\")) + timer_start(1000, (_) => feedkeys("\")) + sleep 1500m + END + CheckScriptSuccess(lines) + nunmap + 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 --- 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 --- 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 // 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 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 // 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 *fu 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