Bug #18516
closed
Memory leak on aliasing method to itself
Added by ibylich (Ilya Bylich) almost 3 years ago.
Updated over 2 years ago.
Description
The following code produces a memory leak:
class A
1.upto(Float::INFINITY) do |i|
define_method(:"foo_#{i}") {}
alias :"foo_#{i}" :"foo_#{i}"
remove_method :"foo_#{i}"
end
end
It is very artificial, but it's required for LSAN/Valgrind integration.
The reason why it leaks is the following:
- alias foo foo increments alias_count even if no alias is created
- later during GC rb_free_method_entry is called that in turn calls rb_method_definition_release that calls free only if alias_count + complemented_count == 0.
- In this particular case alias_count is 1, and so no cleanup happens.
I've been asked to create tickets for memory leaks that I find during LSAN integration if the leak affects previous versions of Ruby. From what I see it does.
Possible fix: https://github.com/ruby/ruby/pull/5492. But like I mentioned in PR I'm not sure if it's correct, @nobu (Nobuyoshi Nakada) mentioned in my main PR aliasing method to itself is used to swallow some warning on method redefinition, but I don't what's the case there.
As @nobu (Nobuyoshi Nakada) mentioned the following code gives no warnings (in verbose mode):
class A
def foo; end
alias foo foo;
def foo; end
end
but if we stop increment alias_count
on alias foo foo
the warning appears (because alias foo foo
becomes a real noop, with absolutely no side effect).
ibylich (Ilya Bylich) wrote in #note-1:
but if we stop increment alias_count
on alias foo foo
the warning appears (because alias foo foo
becomes a real noop, with absolutely no side effect).
Rails heavily depends on this behaviour and if this is changed it will end up raising a lot of warnings. I am not even sure if there is another way to replace methods without raising any warnings, if this is taken away.
- Status changed from Open to Closed
Applied in changeset git|e89d80702bd98a8276243a7fcaa2a158b3bfb659.
Fix memory leak at the same named alias [Bug #18516]
When aliasing a method to the same name method, set a separate bit
flag on that method definition, instead of the reference count
increment. Although this kind of alias has no actual effect at
runtime, is used as the hack to suppress the method re-definition
warning.
- Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN to 2.6: WONTFIX, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: REQUIRED
I took the liberty to set the backport field.
I was able to repro on 2.6, 2.7,3.0 and 3.1. However 2.6 is security only, I don't think it qualifies.
- Backport changed from 2.6: WONTFIX, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: REQUIRED to 2.6: WONTFIX, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: DONE
ruby_3_1 42c9ef769f210d88241a114395dd5ffc27b2fb87 merged revision(s) 7ff1bf317887c0d7b21e91ad548d07b9f05c540c,e89d80702bd98a8276243a7fcaa2a158b3bfb659.
- Backport changed from 2.6: WONTFIX, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: DONE to 2.6: WONTFIX, 2.7: REQUIRED, 3.0: DONE, 3.1: DONE
ruby_3_0 6175823bab28b5d12f66371d67d006df37751fbc merged revision(s) 7ff1bf317887c0d7b21e91ad548d07b9f05c540c,e89d80702bd98a8276243a7fcaa2a158b3bfb659.
- Backport changed from 2.6: WONTFIX, 2.7: REQUIRED, 3.0: DONE, 3.1: DONE to 2.6: WONTFIX, 2.7: DONE, 3.0: DONE, 3.1: DONE
Also available in: Atom
PDF
Like0
Like0Like0Like0Like0Like0Like0Like0Like0