Skip to content

Commit

Permalink
Fix memory leak in complemented method entries
Browse files Browse the repository at this point in the history
[Bug #19894]

When a copy of a complemented method entry is created, there are two
issues:

1. IMEMO_FL_USER3 is not copied, so the complemented status is not
   copied over.
2. In rb_method_entry_clone we increment both alias_count and
   complemented_count. However, when we free the method entry in
   rb_method_definition_release, we only decrement one of the two
   counters, resulting in the rb_method_definition_t being leaked.

Co-authored-by: Adam Hess <adamhess1991@gmail.com>
  • Loading branch information
peterzhu2118 and HParker committed Sep 20, 2023
1 parent 3c11cdb commit 96c5a4b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
5 changes: 3 additions & 2 deletions method.h
Expand Up @@ -101,8 +101,9 @@ static inline void
METHOD_ENTRY_FLAGS_COPY(rb_method_entry_t *dst, const rb_method_entry_t *src)
{
dst->flags =
(dst->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2)) |
(src->flags & (IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2));
(dst->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2
|IMEMO_FL_USER3)) |
(src->flags & (IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2|IMEMO_FL_USER3));
}

typedef enum {
Expand Down
29 changes: 29 additions & 0 deletions test/ruby/test_module.rb
Expand Up @@ -3292,6 +3292,35 @@ def test_iclass_memory_leak
CODE
end

def test_complemented_method_entry_memory_leak
# [Bug #19894]
assert_no_memory_leak([], <<~PREP, <<~CODE, rss: true)
code = proc do
$c = Class.new do
def foo; end
end
$m = Module.new do
refine $c do
def foo; end
end
end
Class.new do
using $m
def initialize
o = $c.new
o.method(:foo).unbind
end
end.new
end
1_000.times(&code)
PREP
100_000.times(&code)
CODE
end

private

def assert_top_method_is_private(method)
Expand Down
8 changes: 5 additions & 3 deletions vm_method.c
Expand Up @@ -693,11 +693,13 @@ rb_method_entry_create(ID called_id, VALUE klass, rb_method_visibility_t visi, c
const rb_method_entry_t *
rb_method_entry_clone(const rb_method_entry_t *src_me)
{
rb_method_entry_t *me = rb_method_entry_alloc(src_me->called_id, src_me->owner, src_me->defined_class,
method_definition_addref(src_me->def));
rb_method_entry_t *me = rb_method_entry_alloc(src_me->called_id, src_me->owner, src_me->defined_class, src_me->def);
if (METHOD_ENTRY_COMPLEMENTED(src_me)) {
method_definition_addref_complement(src_me->def);
}
else {
method_definition_addref(src_me->def);
}

METHOD_ENTRY_FLAGS_COPY(me, src_me);
return me;
Expand All @@ -724,7 +726,7 @@ rb_method_entry_complement_defined_class(const rb_method_entry_t *src_me, ID cal
def = NULL;
}
else {
def = method_definition_addref_complement(def);
method_definition_addref_complement(def);
}
me = rb_method_entry_alloc(called_id, src_me->owner, defined_class, def);
METHOD_ENTRY_FLAGS_COPY(me, src_me);
Expand Down

0 comments on commit 96c5a4b

Please sign in to comment.