Bug #18516
closedMemory leak on aliasing method to itself
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.
Updated by ibylich (Ilya Bylich) almost 3 years ago
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).
Updated by ufuk (Ufuk Kayserilioglu) almost 3 years ago
ibylich (Ilya Bylich) wrote in #note-1:
but if we stop increment
alias_count
onalias foo foo
the warning appears (becausealias 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.
Updated by nobu (Nobuyoshi Nakada) almost 3 years ago
Updated by nobu (Nobuyoshi Nakada) almost 3 years ago
- 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.
Updated by byroot (Jean Boussier) almost 3 years ago
- 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.
Updated by naruse (Yui NARUSE) almost 3 years ago
- 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.
Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago
- 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.
Updated by usa (Usaku NAKAMURA) over 2 years ago
- 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