To: vim_dev@googlegroups.com Subject: Patch 9.0.1035 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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 -- Bram@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 ///