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: #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
index bbfe566fa3f20aa60a36becfc4967e50757d9fe2..b93bc4201d564fc2f796d5fddffb048eace73d11 100644 (file)
--- 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
index 4c8c1cd2682d2efbf2d28f68f8924ce40936ec25..33dbc98b221a3960aca40741a2ca8f295d3caa6a 100644 (file)
--- 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
index 49976e6d8b53e57ff6cdad3d4f49f18730df5068..4f0cf640ea4de61671e92d742eb5595073a12c74 100644 (file)
--- 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
index 9058b29bcfa5b3583cdf9035605d25d61a362deb..a908f1179a02572d72877db5687f3bb39f64ef10 100644 (file)
--- 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
index ea225c11d3f95348ce60b9f9c31705e7a7a7421d..991ebbce1c6f8087a3125f9515517c2c6e1ff4c7 100644 (file)
--- 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);
 }