patch 9.0.1035: object members are not being marked as used
Problem: Object members are not being marked as used, garbage collection
may free them.
Solution: Mark object members as used. Fix reference counting.
diff --git a/src/eval.c b/src/eval.c
index 917e0a8..703e1ea 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -5233,12 +5233,15 @@
* themselves yet, so that it is possible to decrement refcount counters
*/
- // Go through the list of dicts and free items without the copyID.
+ // Go through the list of dicts and free items without this copyID.
did_free |= dict_free_nonref(copyID);
- // Go through the list of lists and free items without the copyID.
+ // Go through the list of lists and free items without this copyID.
did_free |= list_free_nonref(copyID);
+ // Go through the list of objects and free items without this copyID.
+ did_free |= object_free_nonref(copyID);
+
#ifdef FEAT_JOB_CHANNEL
// Go through the list of jobs and free items without the copyID. This
// must happen before doing channels, because jobs refer to channels, but
@@ -5405,7 +5408,8 @@
}
/*
- * Mark all lists and dicts referenced through typval "tv" with "copyID".
+ * Mark all lists, dicts and other container types referenced through typval
+ * "tv" with "copyID".
* "list_stack" is used to add lists to be marked. Can be NULL.
* "ht_stack" is used to add hashtabs to be marked. Can be NULL.
*
@@ -5420,162 +5424,214 @@
{
int abort = FALSE;
- if (tv->v_type == VAR_DICT)
+ switch (tv->v_type)
{
- dict_T *dd = tv->vval.v_dict;
-
- if (dd != NULL && dd->dv_copyID != copyID)
+ case VAR_DICT:
{
- // Didn't see this dict yet.
- dd->dv_copyID = copyID;
- if (ht_stack == NULL)
- {
- abort = set_ref_in_ht(&dd->dv_hashtab, copyID, list_stack);
- }
- else
- {
- ht_stack_T *newitem = ALLOC_ONE(ht_stack_T);
+ dict_T *dd = tv->vval.v_dict;
- if (newitem == NULL)
- abort = TRUE;
+ if (dd != NULL && dd->dv_copyID != copyID)
+ {
+ // Didn't see this dict yet.
+ dd->dv_copyID = copyID;
+ if (ht_stack == NULL)
+ {
+ abort = set_ref_in_ht(&dd->dv_hashtab, copyID, list_stack);
+ }
else
{
- newitem->ht = &dd->dv_hashtab;
- newitem->prev = *ht_stack;
- *ht_stack = newitem;
- }
- }
- }
- }
- else if (tv->v_type == VAR_LIST)
- {
- list_T *ll = tv->vval.v_list;
+ ht_stack_T *newitem = ALLOC_ONE(ht_stack_T);
- if (ll != NULL && ll->lv_copyID != copyID)
- {
- // Didn't see this list yet.
- ll->lv_copyID = copyID;
- if (list_stack == NULL)
- {
- abort = set_ref_in_list_items(ll, copyID, ht_stack);
- }
- else
- {
- list_stack_T *newitem = ALLOC_ONE(list_stack_T);
-
- if (newitem == NULL)
- abort = TRUE;
- else
- {
- newitem->list = ll;
- newitem->prev = *list_stack;
- *list_stack = newitem;
- }
- }
- }
- }
- else if (tv->v_type == VAR_FUNC)
- {
- abort = set_ref_in_func(tv->vval.v_string, NULL, copyID);
- }
- else if (tv->v_type == VAR_PARTIAL)
- {
- partial_T *pt = tv->vval.v_partial;
- int i;
-
- if (pt != NULL && pt->pt_copyID != copyID)
- {
- // Didn't see this partial yet.
- pt->pt_copyID = copyID;
-
- abort = set_ref_in_func(pt->pt_name, pt->pt_func, copyID);
-
- if (pt->pt_dict != NULL)
- {
- typval_T dtv;
-
- dtv.v_type = VAR_DICT;
- dtv.vval.v_dict = pt->pt_dict;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
- }
-
- 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()
- // pt_loopvars is handled in set_ref_in_loopvars()
- }
- }
-#ifdef FEAT_JOB_CHANNEL
- else if (tv->v_type == VAR_JOB)
- {
- job_T *job = tv->vval.v_job;
- typval_T dtv;
-
- if (job != NULL && job->jv_copyID != copyID)
- {
- job->jv_copyID = copyID;
- if (job->jv_channel != NULL)
- {
- dtv.v_type = VAR_CHANNEL;
- dtv.vval.v_channel = job->jv_channel;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
- }
- if (job->jv_exit_cb.cb_partial != NULL)
- {
- dtv.v_type = VAR_PARTIAL;
- dtv.vval.v_partial = job->jv_exit_cb.cb_partial;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
- }
- }
- }
- else if (tv->v_type == VAR_CHANNEL)
- {
- channel_T *ch =tv->vval.v_channel;
- ch_part_T part;
- typval_T dtv;
- jsonq_T *jq;
- cbq_T *cq;
-
- if (ch != NULL && ch->ch_copyID != copyID)
- {
- ch->ch_copyID = copyID;
- for (part = PART_SOCK; part < PART_COUNT; ++part)
- {
- for (jq = ch->ch_part[part].ch_json_head.jq_next; jq != NULL;
- jq = jq->jq_next)
- set_ref_in_item(jq->jq_value, copyID, ht_stack, list_stack);
- for (cq = ch->ch_part[part].ch_cb_head.cq_next; cq != NULL;
- cq = cq->cq_next)
- if (cq->cq_callback.cb_partial != NULL)
+ if (newitem == NULL)
+ abort = TRUE;
+ else
{
- dtv.v_type = VAR_PARTIAL;
- dtv.vval.v_partial = cq->cq_callback.cb_partial;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ newitem->ht = &dd->dv_hashtab;
+ newitem->prev = *ht_stack;
+ *ht_stack = newitem;
}
- if (ch->ch_part[part].ch_callback.cb_partial != NULL)
+ }
+ }
+ break;
+ }
+
+ case VAR_LIST:
+ {
+ list_T *ll = tv->vval.v_list;
+
+ if (ll != NULL && ll->lv_copyID != copyID)
+ {
+ // Didn't see this list yet.
+ ll->lv_copyID = copyID;
+ if (list_stack == NULL)
+ {
+ abort = set_ref_in_list_items(ll, copyID, ht_stack);
+ }
+ else
+ {
+ list_stack_T *newitem = ALLOC_ONE(list_stack_T);
+
+ if (newitem == NULL)
+ abort = TRUE;
+ else
+ {
+ newitem->list = ll;
+ newitem->prev = *list_stack;
+ *list_stack = newitem;
+ }
+ }
+ }
+ break;
+ }
+
+ case VAR_FUNC:
+ {
+ abort = set_ref_in_func(tv->vval.v_string, NULL, copyID);
+ break;
+ }
+
+ case VAR_PARTIAL:
+ {
+ partial_T *pt = tv->vval.v_partial;
+ int i;
+
+ if (pt != NULL && pt->pt_copyID != copyID)
+ {
+ // Didn't see this partial yet.
+ pt->pt_copyID = copyID;
+
+ abort = set_ref_in_func(pt->pt_name, pt->pt_func, copyID);
+
+ if (pt->pt_dict != NULL)
+ {
+ typval_T dtv;
+
+ dtv.v_type = VAR_DICT;
+ dtv.vval.v_dict = pt->pt_dict;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+
+ 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()
+ // pt_loopvars is handled in set_ref_in_loopvars()
+ }
+ break;
+ }
+
+ case VAR_JOB:
+ {
+#ifdef FEAT_JOB_CHANNEL
+ job_T *job = tv->vval.v_job;
+ typval_T dtv;
+
+ if (job != NULL && job->jv_copyID != copyID)
+ {
+ job->jv_copyID = copyID;
+ if (job->jv_channel != NULL)
+ {
+ dtv.v_type = VAR_CHANNEL;
+ dtv.vval.v_channel = job->jv_channel;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+ if (job->jv_exit_cb.cb_partial != NULL)
{
dtv.v_type = VAR_PARTIAL;
- dtv.vval.v_partial =
- ch->ch_part[part].ch_callback.cb_partial;
+ dtv.vval.v_partial = job->jv_exit_cb.cb_partial;
set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
}
}
- if (ch->ch_callback.cb_partial != NULL)
- {
- dtv.v_type = VAR_PARTIAL;
- dtv.vval.v_partial = ch->ch_callback.cb_partial;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
- }
- if (ch->ch_close_cb.cb_partial != NULL)
- {
- dtv.v_type = VAR_PARTIAL;
- dtv.vval.v_partial = ch->ch_close_cb.cb_partial;
- set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
- }
- }
- }
#endif
+ break;
+ }
+
+ case VAR_CHANNEL:
+ {
+#ifdef FEAT_JOB_CHANNEL
+ channel_T *ch = tv->vval.v_channel;
+ ch_part_T part;
+ typval_T dtv;
+ jsonq_T *jq;
+ cbq_T *cq;
+
+ if (ch != NULL && ch->ch_copyID != copyID)
+ {
+ ch->ch_copyID = copyID;
+ for (part = PART_SOCK; part < PART_COUNT; ++part)
+ {
+ for (jq = ch->ch_part[part].ch_json_head.jq_next;
+ jq != NULL; jq = jq->jq_next)
+ set_ref_in_item(jq->jq_value, copyID,
+ ht_stack, list_stack);
+ for (cq = ch->ch_part[part].ch_cb_head.cq_next; cq != NULL;
+ cq = cq->cq_next)
+ if (cq->cq_callback.cb_partial != NULL)
+ {
+ dtv.v_type = VAR_PARTIAL;
+ dtv.vval.v_partial = cq->cq_callback.cb_partial;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+ if (ch->ch_part[part].ch_callback.cb_partial != NULL)
+ {
+ dtv.v_type = VAR_PARTIAL;
+ dtv.vval.v_partial =
+ ch->ch_part[part].ch_callback.cb_partial;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+ }
+ if (ch->ch_callback.cb_partial != NULL)
+ {
+ dtv.v_type = VAR_PARTIAL;
+ dtv.vval.v_partial = ch->ch_callback.cb_partial;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+ if (ch->ch_close_cb.cb_partial != NULL)
+ {
+ dtv.v_type = VAR_PARTIAL;
+ dtv.vval.v_partial = ch->ch_close_cb.cb_partial;
+ set_ref_in_item(&dtv, copyID, ht_stack, list_stack);
+ }
+ }
+#endif
+ break;
+ }
+
+ case VAR_CLASS:
+ // TODO: mark methods in class_obj_methods ?
+ break;
+
+ case VAR_OBJECT:
+ {
+ object_T *obj = tv->vval.v_object;
+ if (obj != NULL && obj->obj_copyID != copyID)
+ {
+ obj->obj_copyID = copyID;
+
+ // The typval_T array is right after the object_T.
+ typval_T *mtv = (typval_T *)(obj + 1);
+ for (int i = 0; !abort
+ && i < obj->obj_class->class_obj_member_count; ++i)
+ abort = abort || set_ref_in_item(mtv + i, copyID,
+ ht_stack, list_stack);
+ }
+ break;
+ }
+
+ case VAR_UNKNOWN:
+ case VAR_ANY:
+ case VAR_VOID:
+ case VAR_BOOL:
+ case VAR_SPECIAL:
+ case VAR_NUMBER:
+ case VAR_FLOAT:
+ case VAR_STRING:
+ case VAR_BLOB:
+ case VAR_INSTR:
+ // Types that do not contain any other item
+ break;
+ }
+
return abort;
}
diff --git a/src/proto/vim9class.pro b/src/proto/vim9class.pro
index 4c6e12d..9a6b23f 100644
--- a/src/proto/vim9class.pro
+++ b/src/proto/vim9class.pro
@@ -8,5 +8,8 @@
void copy_object(typval_T *from, typval_T *to);
void object_unref(object_T *obj);
void copy_class(typval_T *from, typval_T *to);
-void class_unref(typval_T *tv);
+void class_unref(class_T *cl);
+void object_created(object_T *obj);
+void object_cleared(object_T *obj);
+int object_free_nonref(int copyID);
/* vim: set ft=c : */
diff --git a/src/structs.h b/src/structs.h
index 0fb57f4..09b0743 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1487,8 +1487,13 @@
// Used for v_object of typval of VAR_OBJECT.
// The member variables follow in an array of typval_T.
struct object_S {
- class_T *obj_class; // class this object is created for
+ class_T *obj_class; // class this object is created for;
+ // pointer adds to class_refcount
int obj_refcount;
+
+ object_T *obj_next_used; // for list headed by "first_object"
+ object_T *obj_prev_used; // for list headed by "first_object"
+ int obj_copyID; // used by garbage collection
};
#define TTFLAG_VARARGS 0x01 // func args ends with "..."
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index 6742ea7..1945db1 100644
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -132,11 +132,10 @@
this.col: number
endclass
- # # FIXME: this works but leaks memory
- # # use the automatically generated new() method
- # var pos = TextPosition.new(2, 12)
- # assert_equal(2, pos.lnum)
- # assert_equal(12, pos.col)
+ # use the automatically generated new() method
+ var pos = TextPosition.new(2, 12)
+ assert_equal(2, pos.lnum)
+ assert_equal(12, pos.col)
END
v9.CheckScriptSuccess(lines)
enddef
diff --git a/src/typval.c b/src/typval.c
index 6faebe4..98915cc 100644
--- a/src/typval.c
+++ b/src/typval.c
@@ -85,7 +85,7 @@
break;
#endif
case VAR_CLASS:
- class_unref(varp);
+ class_unref(varp->vval.v_class);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
@@ -161,7 +161,7 @@
VIM_CLEAR(varp->vval.v_instr);
break;
case VAR_CLASS:
- class_unref(varp);
+ class_unref(varp->vval.v_class);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
diff --git a/src/version.c b/src/version.c
index f8977bc..7ae34f4 100644
--- a/src/version.c
+++ b/src/version.c
@@ -696,6 +696,8 @@
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 1035,
+/**/
1034,
/**/
1033,
diff --git a/src/vim9class.c b/src/vim9class.c
index 5804b12..864a2e0 100644
--- a/src/vim9class.c
+++ b/src/vim9class.c
@@ -419,13 +419,18 @@
CLEAR_FIELD(funcexe);
funcexe.fe_evaluate = TRUE;
+ // Clear the class or object after calling the function, in
+ // case the refcount is one.
+ typval_T tv_tofree = *rettv;
+ rettv->v_type = VAR_UNKNOWN;
+
// Call the user function. Result goes into rettv;
// TODO: pass the object
- rettv->v_type = VAR_UNKNOWN;
int error = call_user_func_check(fp, argcount, argvars,
rettv, &funcexe, NULL);
- // Clear the arguments.
+ // Clear the previous rettv and the arguments.
+ clear_tv(&tv_tofree);
for (int idx = 0; idx < argcount; ++idx)
clear_tv(&argvars[idx]);
@@ -494,7 +499,11 @@
for (int i = 0; i < cl->class_obj_member_count; ++i)
clear_tv(tv + i);
+ // Remove from the list headed by "first_object".
+ object_cleared(obj);
+
vim_free(obj);
+ class_unref(cl);
}
/*
@@ -522,9 +531,8 @@
* Unreference a class. Free it when the reference count goes down to zero.
*/
void
-class_unref(typval_T *tv)
+class_unref(class_T *cl)
{
- class_T *cl = tv->vval.v_class;
if (cl != NULL && --cl->class_refcount <= 0)
{
vim_free(cl->class_name);
@@ -547,5 +555,60 @@
}
}
+static object_T *first_object = NULL;
+
+/*
+ * Call this function when an object has been created. It will be added to the
+ * list headed by "first_object".
+ */
+ void
+object_created(object_T *obj)
+{
+ if (first_object != NULL)
+ {
+ obj->obj_next_used = first_object;
+ first_object->obj_prev_used = obj;
+ }
+ first_object = obj;
+}
+
+/*
+ * Call this function when an object has been cleared and is about to be freed.
+ * It is removed from the list headed by "first_object".
+ */
+ void
+object_cleared(object_T *obj)
+{
+ if (obj->obj_next_used != NULL)
+ obj->obj_next_used->obj_prev_used = obj->obj_prev_used;
+ if (obj->obj_prev_used != NULL)
+ obj->obj_prev_used->obj_next_used = obj->obj_next_used;
+ else if (first_object == obj)
+ first_object = obj->obj_next_used;
+}
+
+/*
+ * Go through the list of all objects and free items without "copyID".
+ */
+ int
+object_free_nonref(int copyID)
+{
+ int did_free = FALSE;
+ object_T *next_obj;
+
+ for (object_T *obj = first_object; obj != NULL; obj = next_obj)
+ {
+ next_obj = obj->obj_next_used;
+ if ((obj->obj_copyID & COPYID_MASK) != (copyID & COPYID_MASK))
+ {
+ // Free the object and items it contains.
+ object_clear(obj);
+ did_free = TRUE;
+ }
+ }
+
+ return did_free;
+}
+
#endif // FEAT_EVAL
diff --git a/src/vim9execute.c b/src/vim9execute.c
index b164502..3b142b8 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -3018,7 +3018,9 @@
iptr->isn_arg.construct.construct_size);
tv->vval.v_object->obj_class =
iptr->isn_arg.construct.construct_class;
+ ++tv->vval.v_object->obj_class->class_refcount;
tv->vval.v_object->obj_refcount = 1;
+ object_created(tv->vval.v_object);
break;
// execute Ex command line