Project

General

Profile

Actions

Bug #18553

closed

Memory leak on compiling method call with kwargs

Added by ibylich (Ilya Bylich) almost 3 years ago. Updated about 1 year ago.

Status:
Closed
Target version:
-
[ruby-core:107302]

Description

The following code produces a memory leak:

p(foo: 1)

It comes from the allocation in compile.c:

struct rb_callinfo_kwarg *kw_arg =
                rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg));

Later it's packed into struct rb_callinfo, but imemo_callinfo is a GC-managed object that has no free calls in obj_free function in gc.c.

I've tried to fix it by calling free on callinfo->kwarg and it fixed leak in ./miniruby -e 'p(foo: 1)', but it breaks make install. My addition causes a double-free error, looks like either callinfo or callinfo->kwarg is shared between multiple objects.
kwarg field is a const pointer, so that's somewhat expected (I was under impression that const qualifier is redundant) :)

I'm pretty sure old versions of Ruby are also affected by this memory leak.


Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #19907: Method calls with keyword arguments in eval leaks callcache and callinfo objectsClosedActions
Related to Ruby master - Bug #19906: fix kwarg memory leakClosedActions

Updated by jeremyevans0 (Jeremy Evans) over 2 years ago

  • Assignee set to ko1 (Koichi Sasada)

I did some testing and can confirm this is still a bug in the master branch. From my testing, this isn't related to not freeing the struct rb_callinfo_kwarg, it's because compiling a keyword argument method call causes two imemo_callinfo objects to be created, and neither of these is collected unless the method they are calling is removed. Maybe there is a separate bug in regards to not freeing struct rb_callinfo_kwarg?

Here's an example reproducer. This compiles and runs code with 2 keyword argument method calls in a loop with 30,000 iterations. Each 10,000 iterations, it prints the number of T_IMEMO objects after a full garbage collection.

def p(**) end
s = 'p(foo: 1); p(foo: 1)'
30000.times do |i|
  eval(s)
  if i % 10000 == 0
    GC.start
    puts(ObjectSpace.count_objects[:T_IMEMO].to_s)
  end
end
GC.start
puts(ObjectSpace.count_objects[:T_IMEMO].to_s)
Object.send(:remove_method, :p)
GC.start
puts(ObjectSpace.count_objects[:T_IMEMO].to_s)

Here's the output, showing the T_IMEMO objects increasing at ~40,000 per print (2 for each of the 2 keyword method calls, 10000 times), and being retained until the related method is removed:

5003
45007
85007
124988
4993

My uneducated approach to fix this would be to tie imemo_callinfo to the containing iseq and not the method definition. That way, if the instruction sequence is collected, any imemo_callinfo inside it are collected with it. Theoretically, imemo_callinfo is caller information, not callee information, and should be tied to callers (the iseq), not callees (the method). However, I don't know whether that approach is viable, and haven't attempted it yet.

I was also able to determine that this imemo_callinfo issue was introduced in Ruby 3.0, since running the above example on Ruby 2.7 shows that the number of T_IMEMO objects remained roughly the same. I bisected and this bug was introduced in b9007b6c548f91e88fd3f2ffa23de740431fa969 (Introduce disposable call-cache.).

Actions #2

Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

  • Related to Bug #19907: Method calls with keyword arguments in eval leaks callcache and callinfo objects added
Actions #3

Updated by peterzhu2118 (Peter Zhu) about 1 year ago

  • Related to Bug #19906: fix kwarg memory leak added

Updated by peterzhu2118 (Peter Zhu) about 1 year ago

  • Status changed from Open to Closed

This was fixed in #19906, so I will close this issue.

Actions #5

Updated by Anonymous about 1 year ago


Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0