To: vim_dev@googlegroups.com Subject: Patch 8.2.3809 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.3809 Problem: Vim9: crash when garbage collecting a nested partial. (Virginia Senioria) Solution: Set references in all the funcstacks. (closes #9348) Files: src/vim9execute.c, src/proto/vim9execute.pro, src/structs.h, src/eval.c, src/testdir/test_vim9_func.vim *** ../vim-8.2.3808/src/vim9execute.c 2021-12-14 14:29:12.053734514 +0000 --- src/vim9execute.c 2021-12-14 17:58:36.178986528 +0000 *************** *** 468,473 **** --- 468,498 ---- // 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,545 **** --- 565,571 ---- // 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,554 **** --- 575,581 ---- vim_free(funcstack); return FAIL; } + add_funcstack_to_list(funcstack); // Move or copy the arguments. for (idx = 0; idx < argcount; ++idx) *************** *** 571,577 **** // 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. if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL) { int i; --- 598,604 ---- // 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 know when to check. if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL) { int i; *************** *** 643,653 **** --- 670,703 ---- 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 *** ../vim-8.2.3808/src/proto/vim9execute.pro 2021-09-02 17:49:02.744932328 +0100 --- src/proto/vim9execute.pro 2021-12-14 17:47:09.295704754 +0000 *************** *** 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); *** ../vim-8.2.3808/src/structs.h 2021-12-13 14:26:40.992627753 +0000 --- src/structs.h 2021-12-14 17:46:09.915820923 +0000 *************** *** 2009,2016 **** * Structure to hold the context of a compiled function, used by closures * defined in that function. */ ! typedef struct funcstack_S { garray_T fs_ga; // contains the stack, with: // - arguments // - frame --- 2009,2021 ---- * Structure to hold the context of a compiled function, used by closures * defined in that function. */ ! 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,2027 **** 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 { --- 2026,2032 ---- 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 ! }; typedef struct outer_S outer_T; struct outer_S { *** ../vim-8.2.3808/src/eval.c 2021-12-14 12:06:12.533925687 +0000 --- src/eval.c 2021-12-14 17:44:20.900034455 +0000 *************** *** 4510,4515 **** --- 4510,4518 ---- // 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,4883 **** 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); ! } ! } } #ifdef FEAT_JOB_CHANNEL --- 4872,4878 ---- for (i = 0; i < pt->pt_argc; ++i) abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID, ht_stack, list_stack); ! // pt_funcstack is handled in set_ref_in_funcstacks() } } #ifdef FEAT_JOB_CHANNEL *** ../vim-8.2.3808/src/testdir/test_vim9_func.vim 2021-12-13 18:14:11.760881287 +0000 --- src/testdir/test_vim9_func.vim 2021-12-14 18:11:57.841770846 +0000 *************** *** 1903,1908 **** --- 1903,1943 ---- 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 *** ../vim-8.2.3808/src/version.c 2021-12-14 14:29:12.057734501 +0000 --- src/version.c 2021-12-14 18:12:54.005677305 +0000 *************** *** 751,752 **** --- 751,754 ---- { /* Add new patch number below this line */ + /**/ + 3809, /**/ -- hundred-and-one symptoms of being an internet addict: 32. You don't know what sex three of your closest friends are, because they have neutral nicknames and you never bothered to ask. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///