patch 9.1.1566: self-referenced enum may not get freed

yegappan · chrisbra · commit 0e40501a9d1f · 2025-07-18T21:50:20.000+02:00
Problem: self-referenced enum may not get freed Solution: Test if it can be freed (Yegappan Lakshmanan) closes: #17743 Signed-off-by: Yegappan Lakshmanan <yegappan@yahoo.com> Signed-off-by: Christian Brabandt <cb@256bit.org>
diff --git a/src/eval.c b/src/eval.c
@@ -6025,7 +6025,8 @@ partial_free(partial_T *pt)
     }
     else
 	func_ptr_unref(pt->pt_func);
-    object_unref(pt->pt_obj);
+    if (pt->pt_obj != NULL)
+	object_unref(pt->pt_obj);
 
     // "out_up" is no longer used, decrement refcount on partial that owns it.
     partial_unref(pt->pt_outer.out_up_partial);
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
@@ -11032,7 +11032,6 @@ def Test_method_string()
   v9.CheckScriptSuccess(lines)
 enddef
 
-
 " Test for using a class in the class definition
 def Test_Ref_Class_Within_Same_Class()
   var lines =<< trim END
@@ -13152,4 +13151,21 @@ def Test_obj_class_member_type()
   v9.CheckSourceFailure(lines, 'E1013: Argument 2: type mismatch, expected dict<number> but got dict<string> in extend()', 7)
 enddef
 
+" Test for garbage collecting a class with a member referring to the class
+" (self reference)
+func Test_class_selfref_gc()
+  let lines =<< trim END
+    vim9script
+    class Foo
+      static var MyFoo = Foo.new()
+      static var d = {a: [1, 2]}
+      static var l = [{a: 'a', b: 'b'}]
+    endclass
+    assert_equal(2, test_refcount(Foo))
+    test_garbagecollect_now()
+    assert_equal(2, test_refcount(Foo))
+  END
+  call v9.CheckSourceSuccess(lines)
+endfunc
+
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
diff --git a/src/testdir/test_vim9_enum.vim b/src/testdir/test_vim9_enum.vim
@@ -1647,4 +1647,21 @@ def Test_enum_echo()
   v9.CheckScriptSuccess(lines)
 enddef
 
+" Test for garbage collecting an enum with a complex member variables.
+func Test_class_selfref_gc()
+  let lines =<< trim END
+    vim9script
+    enum Foo
+      Red,
+      Blue
+      static var d = {a: [1, 2]}
+      static var l = [{a: 'a', b: 'b'}]
+    endenum
+    assert_equal(3, test_refcount(Foo))
+    test_garbagecollect_now()
+    assert_equal(3, test_refcount(Foo))
+  END
+  call v9.CheckSourceSuccess(lines)
+endfunc
+
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
diff --git a/src/version.c b/src/version.c
@@ -719,6 +719,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1566,
 /**/
     1565,
 /**/
diff --git a/src/vim9class.c b/src/vim9class.c
@@ -3602,6 +3602,10 @@ class_free(class_T *cl)
     for (int i = 0; i < cl->class_class_function_count; ++i)
     {
 	ufunc_T *uf = cl->class_class_functions[i];
+	// For an enum class, the constructor function names are cleared.  Set
+	// the name back, so that clearing the function will work properly.
+	if (IS_ENUM(cl) && IS_CONSTRUCTOR_METHOD(uf) && *uf->uf_name == NUL)
+	    STRCPY(uf->uf_name, "new");
 	func_clear_free(uf, FALSE);
     }
     vim_free(cl->class_class_functions);
@@ -3620,13 +3624,107 @@ class_free(class_T *cl)
     vim_free(cl);
 }
 
+/*
+ * Returns the number of references from the class members of class "cl" to cl"
+ * itself.
+ */
+    static int
+class_get_selfrefs(class_T *cl)
+{
+    int		self_refs = 0;
+    typval_T	*tv;
+
+    for (int i = 0; i < cl->class_class_member_count; ++i)
+    {
+	tv = &cl->class_members_tv[i];
+	if (tv->v_type == VAR_OBJECT && tv->vval.v_object->obj_class == cl
+		&& (tv->vval.v_object->obj_refcount == 1
+		    || (IS_ENUM(cl) && tv->vval.v_object->obj_refcount == 2)))
+	    self_refs++;
+    }
+
+    return self_refs;
+}
+
+/*
+ * Returns TRUE if enum "cl" can be freed.  An enum can be freed, if the enum
+ * values are no longer referenced and the "values" class member is also not
+ * referenced.
+ */
+    static int
+can_free_enum(class_T *cl)
+{
+    for (int i = 0; i < cl->class_class_member_count; ++i)
+    {
+	typval_T	*tv = &cl->class_members_tv[i];
+	ocmember_T	*ocm = &cl->class_class_members[i];
+
+	if (tv->v_type != VAR_OBJECT && tv->v_type != VAR_LIST)
+	    // In an enum, the first set of class members are the enum values.
+	    // Followed by the "values" member which is a List of enum values.
+	    // If all of those members are no longer referenced, then the enum
+	    // may be freed.
+	    return TRUE;
+
+	if (tv->v_type == VAR_LIST
+		&& tv->vval.v_list->lv_type->tt_member->tt_type == VAR_OBJECT
+		&& tv->vval.v_list->lv_type->tt_member->tt_class == cl
+		&& STRCMP(ocm->ocm_name, "values") == 0)
+	{
+	    // "values" class member is referenced outside
+	    if (tv->vval.v_list->lv_refcount > 1)
+		return FALSE;
+	    break;
+	}
+
+	if (tv->vval.v_object->obj_refcount > 2)
+	    // enum value is referenced outside
+	    return FALSE;
+    }
+
+    return TRUE;
+}
+
+/*
+ * Returns TRUE if class "cl" has no references outside of the references from
+ * it's own class members and can be freed.  If it is an enum, then checks
+ * whether the enum values and the "values" members are referenced elsewhere.
+ */
+    static int
+can_free_class(class_T *cl)
+{
+    int		can_free = FALSE;
+    int		self_refs = 0;
+
+    if (IS_ENUM(cl) && !can_free_enum(cl))
+	return FALSE;
+
+    if (cl->class_refcount > 0)
+	self_refs = class_get_selfrefs(cl);
+
+    // If the class is no longer referenced or only referenced from it's own
+    // class member, then it can be freed.
+    if (cl->class_refcount <= 0 || cl->class_refcount == self_refs)
+	can_free = TRUE;
+
+    return can_free;
+}
+
 /*
  * Unreference a class.  Free it when the reference count goes down to zero.
  */
     void
 class_unref(class_T *cl)
 {
-    if (cl != NULL && --cl->class_refcount <= 0 && cl->class_name != NULL)
+    if (cl == NULL)
+	return;
+
+    --cl->class_refcount;
+
+    if (cl->class_name == NULL)
+	return;
+
+    if (can_free_class(cl))
 	class_free(cl);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -6025,7 +6025,8 @@ partial_free(partial_T *pt)`
`6025`	`6025`	`}`
`6026`	`6026`	`else`
`6027`	`6027`	`func_ptr_unref(pt->pt_func);`
`6028`		`- object_unref(pt->pt_obj);`
	`6028`	`+ if (pt->pt_obj != NULL)`
	`6029`	`+ object_unref(pt->pt_obj);`
`6029`	`6030`
`6030`	`6031`	`// "out_up" is no longer used, decrement refcount on partial that owns it.`
`6031`	`6032`	`partial_unref(pt->pt_outer.out_up_partial);`