aniani/vim
0
0
Fork 1
mirror of https://github.com/vim/vim.git synced 2025-09-25 03:54:15 -04:00
Code Activity

patch 9.0.1035: object members are not being marked as used

Browse Source 
Problem:    Object members are not being marked as used, garbage collection
            may free them.
Solution:   Mark object members as used.  Fix reference counting.
This commit is contained in:
Bram Moolenaar
2022-12-08 20:42:00 +00:00
parent e5eae82bb7
commit d28d7b94f5
8 changed files with 288 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

346
src/eval.c
View File

@@ -5233,12 +5233,15 @@ free_unref_items(int copyID)
* themselves yet, so that it is possible to decrement refcount counters * 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); 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); 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 #ifdef FEAT_JOB_CHANNEL
// Go through the list of jobs and free items without the copyID. This // Go through the list of jobs and free items without the copyID. This
// must happen before doing channels, because jobs refer to channels, but // must happen before doing channels, because jobs refer to channels, but
@@ -5405,7 +5408,8 @@ set_ref_in_callback(callback_T *cb, int copyID)
} }
/* /*
* 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. * "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. * "ht_stack" is used to add hashtabs to be marked. Can be NULL.
* *
@@ -5420,162 +5424,214 @@ set_ref_in_item(
{ {
int abort = FALSE; int abort = FALSE;
if (tv->v_type == VAR_DICT) switch (tv->v_type)
{ {
dict_T *dd = tv->vval.v_dict; case VAR_DICT:
if (dd != NULL && dd->dv_copyID != copyID)
{ {
// Didn't see this dict yet. dict_T *dd = tv->vval.v_dict;
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) if (dd != NULL && dd->dv_copyID != copyID)
abort = TRUE; {
// 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 else
{ {
newitem->ht = &dd->dv_hashtab; ht_stack_T *newitem = ALLOC_ONE(ht_stack_T);
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) if (newitem == NULL)
{ abort = TRUE;
// Didn't see this list yet. else
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; newitem->ht = &dd->dv_hashtab;
dtv.vval.v_partial = cq->cq_callback.cb_partial; newitem->prev = *ht_stack;
set_ref_in_item(&dtv, copyID, ht_stack, list_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.v_type = VAR_PARTIAL;
dtv.vval.v_partial = dtv.vval.v_partial = job->jv_exit_cb.cb_partial;
ch->ch_part[part].ch_callback.cb_partial;
set_ref_in_item(&dtv, copyID, ht_stack, list_stack); 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 #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; return abort;
} }

5
src/proto/vim9class.pro
View File

@@ -8,5 +8,8 @@ int class_object_index(char_u **arg, typval_T *rettv, evalarg_T *evalarg, int ve
void copy_object(typval_T *from, typval_T *to); void copy_object(typval_T *from, typval_T *to);
void object_unref(object_T *obj); void object_unref(object_T *obj);
void copy_class(typval_T *from, typval_T *to); 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 : */ /* vim: set ft=c : */

7
src/structs.h
View File

@@ -1487,8 +1487,13 @@ struct class_S
// Used for v_object of typval of VAR_OBJECT. // Used for v_object of typval of VAR_OBJECT.
// The member variables follow in an array of typval_T. // The member variables follow in an array of typval_T.
struct object_S { 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; 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 "..." #define TTFLAG_VARARGS 0x01 // func args ends with "..."

9
src/testdir/test_vim9_class.vim
View File

@@ -132,11 +132,10 @@ def Test_class_basic()
this.col: number this.col: number
endclass endclass
# # FIXME: this works but leaks memory # use the automatically generated new() method
# # use the automatically generated new() method var pos = TextPosition.new(2, 12)
# var pos = TextPosition.new(2, 12) assert_equal(2, pos.lnum)
# assert_equal(2, pos.lnum) assert_equal(12, pos.col)
# assert_equal(12, pos.col)
END END
v9.CheckScriptSuccess(lines) v9.CheckScriptSuccess(lines)
enddef enddef

4
src/typval.c
View File

@@ -85,7 +85,7 @@ free_tv(typval_T *varp)
break; break;
#endif #endif
case VAR_CLASS: case VAR_CLASS:
class_unref(varp); class_unref(varp->vval.v_class);
break; break;
case VAR_OBJECT: case VAR_OBJECT:
object_unref(varp->vval.v_object); object_unref(varp->vval.v_object);
@@ -161,7 +161,7 @@ clear_tv(typval_T *varp)
VIM_CLEAR(varp->vval.v_instr); VIM_CLEAR(varp->vval.v_instr);
break; break;
case VAR_CLASS: case VAR_CLASS:
class_unref(varp); class_unref(varp->vval.v_class);
break; break;
case VAR_OBJECT: case VAR_OBJECT:
object_unref(varp->vval.v_object); object_unref(varp->vval.v_object);

2
src/version.c
View File

@@ -695,6 +695,8 @@ static char *(features[]) =
static int included_patches[] = static int included_patches[] =
{ /* Add new patch number below this line */ { /* Add new patch number below this line */
/**/
1035,
/**/ /**/
1034, 1034,
/**/ /**/

71
src/vim9class.c
View File

@@ -419,13 +419,18 @@ class_object_index(
CLEAR_FIELD(funcexe); CLEAR_FIELD(funcexe);
funcexe.fe_evaluate = TRUE; 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; // Call the user function. Result goes into rettv;
// TODO: pass the object // TODO: pass the object
rettv->v_type = VAR_UNKNOWN;
int error = call_user_func_check(fp, argcount, argvars, int error = call_user_func_check(fp, argcount, argvars,
rettv, &funcexe, NULL); rettv, &funcexe, NULL);
// Clear the arguments. // Clear the previous rettv and the arguments.
clear_tv(&tv_tofree);
for (int idx = 0; idx < argcount; ++idx) for (int idx = 0; idx < argcount; ++idx)
clear_tv(&argvars[idx]); clear_tv(&argvars[idx]);
@@ -494,7 +499,11 @@ object_clear(object_T *obj)
for (int i = 0; i < cl->class_obj_member_count; ++i) for (int i = 0; i < cl->class_obj_member_count; ++i)
clear_tv(tv + i); clear_tv(tv + i);
// Remove from the list headed by "first_object".
object_cleared(obj);
vim_free(obj); vim_free(obj);
class_unref(cl);
} }
/* /*
@@ -522,9 +531,8 @@ copy_class(typval_T *from, typval_T *to)
* Unreference a class. Free it when the reference count goes down to zero. * Unreference a class. Free it when the reference count goes down to zero.
*/ */
void 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) if (cl != NULL && --cl->class_refcount <= 0)
{ {
vim_free(cl->class_name); vim_free(cl->class_name);
@@ -547,5 +555,60 @@ class_unref(typval_T *tv)
} }
} }
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 #endif // FEAT_EVAL

2
src/vim9execute.c
View File

@@ -3018,7 +3018,9 @@ exec_instructions(ectx_T *ectx)
iptr->isn_arg.construct.construct_size); iptr->isn_arg.construct.construct_size);
tv->vval.v_object->obj_class = tv->vval.v_object->obj_class =
iptr->isn_arg.construct.construct_class; iptr->isn_arg.construct.construct_class;
++tv->vval.v_object->obj_class->class_refcount;
tv->vval.v_object->obj_refcount = 1; tv->vval.v_object->obj_refcount = 1;
object_created(tv->vval.v_object);
break; break;
// execute Ex command line // execute Ex command line