Skip to content

Commit

Permalink
Use reference counting to avoid memory leak in kwargs
Browse files Browse the repository at this point in the history
Tracks other callinfo that references the same kwargs and frees them when all references are cleared.

[bug #19906]

Co-authored-by: Peter Zhu <peter@peterzhu.ca>
  • Loading branch information
HParker and peterzhu2118 committed Oct 1, 2023
1 parent be09c83 commit c74dc8b
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 2 deletions.
4 changes: 4 additions & 0 deletions compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -4327,6 +4327,7 @@ compile_keyword_arg(rb_iseq_t *iseq, LINK_ANCHOR *const ret,
rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));
VALUE *keywords = kw_arg->keywords;
int i = 0;
kw_arg->references = 0;
kw_arg->keyword_len = len;

*kw_arg_ptr = kw_arg;
Expand Down Expand Up @@ -7275,6 +7276,7 @@ compile_case3(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const orig_no
fin = NEW_LABEL(line);

kw_arg = rb_xmalloc_mul_add(2, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));
kw_arg->references = 0;
kw_arg->keyword_len = 2;
kw_arg->keywords[0] = ID2SYM(rb_intern("matchee"));
kw_arg->keywords[1] = ID2SYM(rb_intern("key"));
Expand Down Expand Up @@ -10469,6 +10471,7 @@ iseq_build_callinfo_from_hash(rb_iseq_t *iseq, VALUE op)
size_t n = rb_callinfo_kwarg_bytes(len);

kw_arg = xmalloc(n);
kw_arg->references = 0;
kw_arg->keyword_len = len;
for (i = 0; i < len; i++) {
VALUE kw = RARRAY_AREF(vkw_arg, i);
Expand Down Expand Up @@ -11957,6 +11960,7 @@ ibf_load_ci_entries(const struct ibf_load *load,
int kwlen = (int)ibf_load_small_value(load, &reading_pos);
if (kwlen > 0) {
kwarg = rb_xmalloc_mul_add(kwlen, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));
kwarg->references = 0;
kwarg->keyword_len = kwlen;
for (int j=0; j<kwlen; j++) {
VALUE keyword = ibf_load_small_value(load, &reading_pos);
Expand Down
11 changes: 9 additions & 2 deletions gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3681,8 +3681,15 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
RB_DEBUG_COUNTER_INC(obj_imemo_parser_strterm);
break;
case imemo_callinfo:
RB_DEBUG_COUNTER_INC(obj_imemo_callinfo);
break;
{
const struct rb_callinfo * ci = ((const struct rb_callinfo *)obj);
if (ci->kwarg) {
((struct rb_callinfo_kwarg *)ci->kwarg)->references--;
if (ci->kwarg->references == 0) xfree((void *)ci->kwarg);
}
RB_DEBUG_COUNTER_INC(obj_imemo_callinfo);
break;
}
case imemo_callcache:
RB_DEBUG_COUNTER_INC(obj_imemo_callcache);
break;
Expand Down
1 change: 1 addition & 0 deletions rjit_c.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,7 @@ def C.rb_callinfo_kwarg
@rb_callinfo_kwarg ||= CType::Struct.new(
"rb_callinfo_kwarg", Primitive.cexpr!("SIZEOF(struct rb_callinfo_kwarg)"),
keyword_len: [CType::Immediate.parse("int"), Primitive.cexpr!("OFFSETOF((*((struct rb_callinfo_kwarg *)NULL)), keyword_len)")],
references: [CType::Immediate.parse("int"), Primitive.cexpr!("OFFSETOF((*((struct rb_callinfo_kwarg *)NULL)), references)")],
keywords: [CType::Immediate.parse("void *"), Primitive.cexpr!("OFFSETOF((*((struct rb_callinfo_kwarg *)NULL)), keywords)")],
)
end
Expand Down
4 changes: 4 additions & 0 deletions vm_callinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum vm_call_flag_bits {

struct rb_callinfo_kwarg {
int keyword_len;
int references;
VALUE keywords[];
};

Expand Down Expand Up @@ -199,6 +200,9 @@ vm_ci_dump(const struct rb_callinfo *ci)
static inline const struct rb_callinfo *
vm_ci_new_(ID mid, unsigned int flag, unsigned int argc, const struct rb_callinfo_kwarg *kwarg, const char *file, int line)
{
if (kwarg) {
((struct rb_callinfo_kwarg *)kwarg)->references++;
}
if (USE_EMBED_CI && VM_CI_EMBEDDABLE_P(mid, flag, argc, kwarg)) {
RB_DEBUG_COUNTER_INC(ci_packed);
return vm_ci_new_id(mid, flag, argc, kwarg);
Expand Down

0 comments on commit c74dc8b

Please sign in to comment.