Problem: Vim9: Memory leak when using a closure.
Solution: Compute the mininal refcount in the funcstack. Reenable disabled
tests.
else
func_ptr_unref(pt->pt_func);
+ // Decrease the reference count for the context of a closure. If down
+ // to the minimum it may be time to free it.
if (pt->pt_funcstack != NULL)
{
- // Decrease the reference count for the context of a closure. If down
- // to zero free it and clear the variables on the stack.
- if (--pt->pt_funcstack->fs_refcount == 0)
- {
- garray_T *gap = &pt->pt_funcstack->fs_ga;
- typval_T *stack = gap->ga_data;
-
- for (i = 0; i < gap->ga_len; ++i)
- clear_tv(stack + i);
- ga_clear(gap);
- vim_free(pt->pt_funcstack);
- }
- pt->pt_funcstack = NULL;
+ --pt->pt_funcstack->fs_refcount;
+ funcstack_check_refcount(pt->pt_funcstack);
}
vim_free(pt);
void
partial_unref(partial_T *pt)
{
- if (pt != NULL && --pt->pt_refcount <= 0)
- partial_free(pt);
+ if (pt != NULL)
+ {
+ if (--pt->pt_refcount <= 0)
+ partial_free(pt);
+
+ // If the reference count goes down to one, the funcstack may be the
+ // only reference and can be freed if no other partials reference it.
+ else if (pt->pt_refcount == 1 && pt->pt_funcstack != NULL)
+ funcstack_check_refcount(pt->pt_funcstack);
+ }
}
/*
/* vim9execute.c */
void to_string_error(vartype_T vartype);
+void funcstack_check_refcount(funcstack_T *funcstack);
int call_def_function(ufunc_T *ufunc, int argc_arg, typval_T *argv, partial_T *partial, typval_T *rettv);
void ex_disassemble(exarg_T *eap);
int tv2bool(typval_T *tv);
// - arguments
// - frame
// - local variables
+ int fs_var_offset; // count of arguments + frame size == offset to
+ // local variables
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;
res)
enddef
-" TODO: fix memory leak and enable again
-"def s:CreateRefs()
-" var local = 'a'
-" def Append(arg: string)
-" local ..= arg
-" enddef
-" g:Append = Append
-" def Get(): string
-" return local
-" enddef
-" g:Get = Get
-"enddef
-"
-"def Test_disassemble_closure()
-" CreateRefs()
-" var res = execute('disass g:Append')
-" assert_match('<lambda>\d\_s*' ..
-" 'local ..= arg\_s*' ..
-" '\d LOADOUTER $0\_s*' ..
-" '\d LOAD arg\[-1\]\_s*' ..
-" '\d CONCAT\_s*' ..
-" '\d STOREOUTER $0\_s*' ..
-" '\d PUSHNR 0\_s*' ..
-" '\d RETURN',
-" res)
-"
-" res = execute('disass g:Get')
-" assert_match('<lambda>\d\_s*' ..
-" 'return local\_s*' ..
-" '\d LOADOUTER $0\_s*' ..
-" '\d RETURN',
-" res)
-"
-" unlet g:Append
-" unlet g:Get
-"enddef
+
+def s:CreateRefs()
+ var local = 'a'
+ def Append(arg: string)
+ local ..= arg
+ enddef
+ g:Append = Append
+ def Get(): string
+ return local
+ enddef
+ g:Get = Get
+enddef
+
+def Test_disassemble_closure()
+ CreateRefs()
+ var res = execute('disass g:Append')
+ assert_match('<lambda>\d\_s*' ..
+ 'local ..= arg\_s*' ..
+ '\d LOADOUTER $0\_s*' ..
+ '\d LOAD arg\[-1\]\_s*' ..
+ '\d CONCAT\_s*' ..
+ '\d STOREOUTER $0\_s*' ..
+ '\d PUSHNR 0\_s*' ..
+ '\d RETURN',
+ res)
+
+ res = execute('disass g:Get')
+ assert_match('<lambda>\d\_s*' ..
+ 'return local\_s*' ..
+ '\d LOADOUTER $0\_s*' ..
+ '\d RETURN',
+ res)
+
+ unlet g:Append
+ unlet g:Get
+enddef
def EchoArg(arg: string): string
unlet g:UseVararg
enddef
-" TODO: reenable after fixing memory leak
-"def MakeGetAndAppendRefs()
-" var local = 'a'
-"
-" def Append(arg: string)
-" local ..= arg
-" enddef
-" g:Append = Append
-"
-" def Get(): string
-" return local
-" enddef
-" g:Get = Get
-"enddef
-"
-"def Test_closure_append_get()
-" MakeGetAndAppendRefs()
-" g:Get()->assert_equal('a')
-" g:Append('-b')
-" g:Get()->assert_equal('a-b')
-" g:Append('-c')
-" g:Get()->assert_equal('a-b-c')
-"
-" unlet g:Append
-" unlet g:Get
-"enddef
+def MakeGetAndAppendRefs()
+ var local = 'a'
+
+ def Append(arg: string)
+ local ..= arg
+ enddef
+ g:Append = Append
+
+ def Get(): string
+ return local
+ enddef
+ g:Get = Get
+enddef
+
+def Test_closure_append_get()
+ MakeGetAndAppendRefs()
+ g:Get()->assert_equal('a')
+ g:Append('-b')
+ g:Get()->assert_equal('a-b')
+ g:Append('-c')
+ g:Get()->assert_equal('a-b-c')
+
+ unlet g:Append
+ unlet g:Get
+enddef
def Test_nested_closure()
var local = 'text'
CheckScriptSuccess(lines)
enddef
-" TODO: reenable after fixing memory leak
-"def Test_nested_closure_used()
-" var lines =<< trim END
-" vim9script
-" def Func()
-" var x = 'hello'
-" var Closure = {-> x}
-" g:Myclosure = {-> Closure()}
-" enddef
-" Func()
-" assert_equal('hello', g:Myclosure())
-" END
-" CheckScriptSuccess(lines)
-"enddef
+def Test_nested_closure_used()
+ var lines =<< trim END
+ vim9script
+ def Func()
+ var x = 'hello'
+ var Closure = {-> x}
+ g:Myclosure = {-> Closure()}
+ enddef
+ Func()
+ assert_equal('hello', g:Myclosure())
+ END
+ CheckScriptSuccess(lines)
+enddef
def Test_nested_closure_fails()
var lines =<< trim END
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 1819,
/**/
1818,
/**/
// Move them to the called function.
if (funcstack == NULL)
return FAIL;
- funcstack->fs_ga.ga_len = argcount + STACK_FRAME_SIZE
- + dfunc->df_varcount;
+ 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);
funcstack->fs_ga.ga_data = stack;
if (stack == NULL)
{
tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE + idx);
- // Do not copy a partial created for a local function.
- // TODO: This won't work if the closure actually uses it. But when
- // keeping it it gets complicated: it will create a reference cycle
- // inside the partial, thus needs special handling for garbage
- // collection.
- // For now, decide on the reference count.
+ // A partial created for a local function, that is also used as a
+ // 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;
+ int i;
for (i = 0; i < closure_count; ++i)
- {
- partial_T *pt = ((partial_T **)gap->ga_data)[gap->ga_len
- - closure_count + i];
-
- if (tv->vval.v_partial == pt && pt->pt_refcount < 2)
- break;
- }
- if (i < closure_count)
- continue;
+ if (tv->vval.v_partial == ((partial_T **)gap->ga_data)[
+ gap->ga_len - closure_count + i])
+ ++funcstack->fs_min_refcount;
}
- *(stack + argcount + STACK_FRAME_SIZE + idx) = *tv;
+ *(stack + funcstack->fs_var_offset + idx) = *tv;
tv->v_type = VAR_UNKNOWN;
}
return OK;
}
+/*
+ * Called when a partial is freed or its reference count goes down to one. The
+ * funcstack may be the only reference to the partials in the local variables.
+ * Go over all of them, the funcref and can be freed if all partials
+ * referencing the funcstack have a reference count of one.
+ */
+ void
+funcstack_check_refcount(funcstack_T *funcstack)
+{
+ int i;
+ garray_T *gap = &funcstack->fs_ga;
+ int done = 0;
+
+ if (funcstack->fs_refcount > funcstack->fs_min_refcount)
+ return;
+ for (i = funcstack->fs_var_offset; i < gap->ga_len; ++i)
+ {
+ typval_T *tv = ((typval_T *)gap->ga_data) + i;
+
+ if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL
+ && tv->vval.v_partial->pt_funcstack == funcstack
+ && tv->vval.v_partial->pt_refcount == 1)
+ ++done;
+ }
+ if (done == funcstack->fs_min_refcount)
+ {
+ typval_T *stack = gap->ga_data;
+
+ // All partials referencing the funcstack have a reference count of
+ // one, thus the funcstack is no longer of use.
+ for (i = 0; i < gap->ga_len; ++i)
+ clear_tv(stack + i);
+ vim_free(stack);
+ vim_free(funcstack);
+ }
+}
+
/*
* Return from the current function.
*/
projects / vim / commitdiff