From: "jeremyevans0 (Jeremy Evans)" Date: 2022-03-23T21:34:00+00:00 Subject: [ruby-core:108045] [Ruby master Bug#18553] Memory leak on compiling method call with kwargs Issue #18553 has been updated by jeremyevans0 (Jeremy Evans). 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. ```ruby 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 commit:b9007b6c548f91e88fd3f2ffa23de740431fa969 (`Introduce disposable call-cache.`). ---------------------------------------- Bug #18553: Memory leak on compiling method call with kwargs https://bugs.ruby-lang.org/issues/18553#change-97005 * Author: ibylich (Ilya Bylich) * Status: Open * Priority: Normal * Assignee: ko1 (Koichi Sasada) * ruby -v: ruby 3.2.0dev * Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- The following code produces a memory leak: ```ruby p(foo: 1) ``` It comes from the allocation in `compile.c`: ```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](https://github.com/ruby/ruby/pull/5488/files#diff-d1cee85c3b0e24a64519c11150abe26fd0b5d8628a23d356dd0b535ac4595d49R3397) 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. -- https://bugs.ruby-lang.org/ Unsubscribe: