Skip to content

Commit 0e40501

Browse files
yegappanchrisbra
authored andcommitted
patch 9.1.1566: self-referenced enum may not get freed
Problem: self-referenced enum may not get freed Solution: Test if it can be freed (Yegappan Lakshmanan) closes: vim#17743 Signed-off-by: Yegappan Lakshmanan <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>
1 parent f412241 commit 0e40501

File tree

5 files changed

+137
-3
lines changed

5 files changed

+137
-3
lines changed

src/eval.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6025,7 +6025,8 @@ partial_free(partial_T *pt)
60256025
}
60266026
else
60276027
func_ptr_unref(pt->pt_func);
6028-
object_unref(pt->pt_obj);
6028+
if (pt->pt_obj != NULL)
6029+
object_unref(pt->pt_obj);
60296030

60306031
// "out_up" is no longer used, decrement refcount on partial that owns it.
60316032
partial_unref(pt->pt_outer.out_up_partial);

src/testdir/test_vim9_class.vim

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11032,7 +11032,6 @@ def Test_method_string()
1103211032
v9.CheckScriptSuccess(lines)
1103311033
enddef
1103411034

11035-
1103611035
" Test for using a class in the class definition
1103711036
def Test_Ref_Class_Within_Same_Class()
1103811037
var lines =<< trim END
@@ -13152,4 +13151,21 @@ def Test_obj_class_member_type()
1315213151
v9.CheckSourceFailure(lines, 'E1013: Argument 2: type mismatch, expected dict<number> but got dict<string> in extend()', 7)
1315313152
enddef
1315413153

13154+
" Test for garbage collecting a class with a member referring to the class
13155+
" (self reference)
13156+
func Test_class_selfref_gc()
13157+
let lines =<< trim END
13158+
vim9script
13159+
class Foo
13160+
static var MyFoo = Foo.new()
13161+
static var d = {a: [1, 2]}
13162+
static var l = [{a: 'a', b: 'b'}]
13163+
endclass
13164+
assert_equal(2, test_refcount(Foo))
13165+
test_garbagecollect_now()
13166+
assert_equal(2, test_refcount(Foo))
13167+
END
13168+
call v9.CheckSourceSuccess(lines)
13169+
endfunc
13170+
1315513171
" vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

src/testdir/test_vim9_enum.vim

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,4 +1647,21 @@ def Test_enum_echo()
16471647
v9.CheckScriptSuccess(lines)
16481648
enddef
16491649

1650+
" Test for garbage collecting an enum with a complex member variables.
1651+
func Test_class_selfref_gc()
1652+
let lines =<< trim END
1653+
vim9script
1654+
enum Foo
1655+
Red,
1656+
Blue
1657+
static var d = {a: [1, 2]}
1658+
static var l = [{a: 'a', b: 'b'}]
1659+
endenum
1660+
assert_equal(3, test_refcount(Foo))
1661+
test_garbagecollect_now()
1662+
assert_equal(3, test_refcount(Foo))
1663+
END
1664+
call v9.CheckSourceSuccess(lines)
1665+
endfunc
1666+
16501667
" vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

src/version.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,8 @@ static char *(features[]) =
719719

720720
static int included_patches[] =
721721
{ /* Add new patch number below this line */
722+
/**/
723+
1566,
722724
/**/
723725
1565,
724726
/**/

src/vim9class.c

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3602,6 +3602,10 @@ class_free(class_T *cl)
36023602
for (int i = 0; i < cl->class_class_function_count; ++i)
36033603
{
36043604
ufunc_T *uf = cl->class_class_functions[i];
3605+
// For an enum class, the constructor function names are cleared. Set
3606+
// the name back, so that clearing the function will work properly.
3607+
if (IS_ENUM(cl) && IS_CONSTRUCTOR_METHOD(uf) && *uf->uf_name == NUL)
3608+
STRCPY(uf->uf_name, "new");
36053609
func_clear_free(uf, FALSE);
36063610
}
36073611
vim_free(cl->class_class_functions);
@@ -3620,13 +3624,107 @@ class_free(class_T *cl)
36203624
vim_free(cl);
36213625
}
36223626

3627+
/*
3628+
* Returns the number of references from the class members of class "cl" to cl"
3629+
* itself.
3630+
*/
3631+
static int
3632+
class_get_selfrefs(class_T *cl)
3633+
{
3634+
int self_refs = 0;
3635+
typval_T *tv;
3636+
3637+
for (int i = 0; i < cl->class_class_member_count; ++i)
3638+
{
3639+
tv = &cl->class_members_tv[i];
3640+
if (tv->v_type == VAR_OBJECT && tv->vval.v_object->obj_class == cl
3641+
&& (tv->vval.v_object->obj_refcount == 1
3642+
|| (IS_ENUM(cl) && tv->vval.v_object->obj_refcount == 2)))
3643+
self_refs++;
3644+
}
3645+
3646+
return self_refs;
3647+
}
3648+
3649+
/*
3650+
* Returns TRUE if enum "cl" can be freed. An enum can be freed, if the enum
3651+
* values are no longer referenced and the "values" class member is also not
3652+
* referenced.
3653+
*/
3654+
static int
3655+
can_free_enum(class_T *cl)
3656+
{
3657+
for (int i = 0; i < cl->class_class_member_count; ++i)
3658+
{
3659+
typval_T *tv = &cl->class_members_tv[i];
3660+
ocmember_T *ocm = &cl->class_class_members[i];
3661+
3662+
if (tv->v_type != VAR_OBJECT && tv->v_type != VAR_LIST)
3663+
// In an enum, the first set of class members are the enum values.
3664+
// Followed by the "values" member which is a List of enum values.
3665+
// If all of those members are no longer referenced, then the enum
3666+
// may be freed.
3667+
return TRUE;
3668+
3669+
if (tv->v_type == VAR_LIST
3670+
&& tv->vval.v_list->lv_type->tt_member->tt_type == VAR_OBJECT
3671+
&& tv->vval.v_list->lv_type->tt_member->tt_class == cl
3672+
&& STRCMP(ocm->ocm_name, "values") == 0)
3673+
{
3674+
// "values" class member is referenced outside
3675+
if (tv->vval.v_list->lv_refcount > 1)
3676+
return FALSE;
3677+
break;
3678+
}
3679+
3680+
if (tv->vval.v_object->obj_refcount > 2)
3681+
// enum value is referenced outside
3682+
return FALSE;
3683+
}
3684+
3685+
return TRUE;
3686+
}
3687+
3688+
/*
3689+
* Returns TRUE if class "cl" has no references outside of the references from
3690+
* it's own class members and can be freed. If it is an enum, then checks
3691+
* whether the enum values and the "values" members are referenced elsewhere.
3692+
*/
3693+
static int
3694+
can_free_class(class_T *cl)
3695+
{
3696+
int can_free = FALSE;
3697+
int self_refs = 0;
3698+
3699+
if (IS_ENUM(cl) && !can_free_enum(cl))
3700+
return FALSE;
3701+
3702+
if (cl->class_refcount > 0)
3703+
self_refs = class_get_selfrefs(cl);
3704+
3705+
// If the class is no longer referenced or only referenced from it's own
3706+
// class member, then it can be freed.
3707+
if (cl->class_refcount <= 0 || cl->class_refcount == self_refs)
3708+
can_free = TRUE;
3709+
3710+
return can_free;
3711+
}
3712+
36233713
/*
36243714
* Unreference a class. Free it when the reference count goes down to zero.
36253715
*/
36263716
void
36273717
class_unref(class_T *cl)
36283718
{
3629-
if (cl != NULL && --cl->class_refcount <= 0 && cl->class_name != NULL)
3719+
if (cl == NULL)
3720+
return;
3721+
3722+
--cl->class_refcount;
3723+
3724+
if (cl->class_name == NULL)
3725+
return;
3726+
3727+
if (can_free_class(cl))
36303728
class_free(cl);
36313729
}
36323730

0 commit comments

Comments
 (0)