Patch 9.0.1035

[Bram Moolenaar]

Patch 9.0.1035
Problem: Object members are not being marked as used, garbage collection
may free them.
Solution: Mark object members as used. Fix reference counting.
Files: src/eval.c, src/structs.h, src/typval.c, src/vim9class.c,
src/proto/vim9class.pro, src/vim9execute.c,
src/testdir/test_vim9_class.vim


*** ../vim-9.0.1034/src/eval.c 2022-12-08 15:32:11.079034172 +0000
--- src/eval.c 2022-12-08 20:22:18.581287441 +0000
***************
*** 5233,5244 ****
* themselves yet, so that it is possible to decrement refcount counters
*/

! // Go through the list of dicts and free items without the copyID.
did_free |= dict_free_nonref(copyID);

! // Go through the list of lists and free items without the copyID.
did_free |= list_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
--- 5233,5247 ----
* themselves yet, so that it is possible to decrement refcount counters
*/

! // 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 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,5411 ****
}

/*
! * Mark all lists and dicts 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.
*
--- 5408,5415 ----
}

/*
! * 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,5581 ****
{
int abort = FALSE;

! if (tv->v_type == VAR_DICT)
{
! dict_T *dd = tv->vval.v_dict;
!
! 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
! {
! ht_stack_T *newitem = ALLOC_ONE(ht_stack_T);

! if (newitem == NULL)
! abort = TRUE;
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;

! 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)
{
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
return abort;
}

--- 5424,5637 ----
{
int abort = FALSE;

! switch (tv->v_type)
{
! case VAR_DICT:
{
! dict_T *dd = tv->vval.v_dict;

! 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
{
! ht_stack_T *newitem = ALLOC_ONE(ht_stack_T);
!
! if (newitem == NULL)
! abort = TRUE;
! else
! {
! newitem->ht = &dd->dv_hashtab;
! newitem->prev = *ht_stack;
! *ht_stack = newitem;
! }
}
}
+ 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 = job->jv_exit_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;
}

*** ../vim-9.0.1034/src/structs.h 2022-12-08 16:09:57.936588830 +0000
--- src/structs.h 2022-12-08 20:18:19.397183544 +0000
***************
*** 1487,1494 ****
// 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
int obj_refcount;
};

#define TTFLAG_VARARGS 0x01 // func args ends with "..."
--- 1487,1499 ----
// 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;
! // 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 "..."
*** ../vim-9.0.1034/src/typval.c 2022-12-08 15:32:11.083034191 +0000
--- src/typval.c 2022-12-08 20:25:33.425388325 +0000
***************
*** 85,91 ****
break;
#endif
case VAR_CLASS:
! class_unref(varp);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
--- 85,91 ----
break;
#endif
case VAR_CLASS:
! class_unref(varp->vval.v_class);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
***************
*** 161,167 ****
VIM_CLEAR(varp->vval.v_instr);
break;
case VAR_CLASS:
! class_unref(varp);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
--- 161,167 ----
VIM_CLEAR(varp->vval.v_instr);
break;
case VAR_CLASS:
! class_unref(varp->vval.v_class);
break;
case VAR_OBJECT:
object_unref(varp->vval.v_object);
*** ../vim-9.0.1034/src/vim9class.c 2022-12-08 15:32:11.083034191 +0000
--- src/vim9class.c 2022-12-08 20:28:23.013484141 +0000
***************
*** 419,431 ****
CLEAR_FIELD(funcexe);
funcexe.fe_evaluate = TRUE;

// 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.
for (int idx = 0; idx < argcount; ++idx)
clear_tv(&argvars[idx]);

--- 419,436 ----
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
int error = call_user_func_check(fp, argcount, argvars,
rettv, &funcexe, NULL);

! // Clear the previous rettv and the arguments.
! clear_tv(&tv_tofree);
for (int idx = 0; idx < argcount; ++idx)
clear_tv(&argvars[idx]);

***************
*** 494,500 ****
--- 499,509 ----
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,530 ****
* Unreference a class. Free it when the reference count goes down to zero.
*/
void
! class_unref(typval_T *tv)
{
- class_T *cl = tv->vval.v_class;
if (cl != NULL && --cl->class_refcount <= 0)
{
vim_free(cl->class_name);
--- 531,538 ----
* 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)
{
vim_free(cl->class_name);
***************
*** 547,551 ****
--- 555,614 ----
}
}

+ 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
*** ../vim-9.0.1034/src/proto/vim9class.pro 2022-12-08 15:32:11.083034191 +0000
--- src/proto/vim9class.pro 2022-12-08 20:25:20.881381510 +0000
***************
*** 8,12 ****
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);
/* vim: set ft=c : */
--- 8,15 ----
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(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 : */
*** ../vim-9.0.1034/src/vim9execute.c 2022-12-08 15:32:11.083034191 +0000
--- src/vim9execute.c 2022-12-08 20:28:51.085500568 +0000
***************
*** 3018,3024 ****
--- 3018,3026 ----
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
*** ../vim-9.0.1034/src/testdir/test_vim9_class.vim 2022-12-08 15:32:11.087034211 +0000
--- src/testdir/test_vim9_class.vim 2022-12-08 20:41:06.329336578 +0000
***************
*** 132,142 ****
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)
END
v9.CheckScriptSuccess(lines)
enddef
--- 132,141 ----
this.col: number
endclass

! # 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
*** ../vim-9.0.1034/src/version.c 2022-12-08 16:30:13.147504028 +0000
--- src/version.c 2022-12-08 20:26:31.389420377 +0000
***************
*** 697,698 ****
--- 697,700 ----
{ /* Add new patch number below this line */
+ /**/
+ 1035,
/**/

--
hundred-and-one symptoms of being an internet addict:
271. You collect hilarious signatures from all 250 mailing lists you
are subscribed to.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///