[ruby-core:107302] [Ruby master Bug#18553] Memory leak on compiling method call with kwargs

[ruby-core:107302] [Ruby master Bug#18553] Memory leak on compiling method call with kwargs

[ibylich (Ilya Bylich)]

Issue #18553 has been reported by ibylich (Ilya Bylich).

----------------------------------------
Bug #18553: Memory leak on compiling method call with kwargs
https://bugs.ruby-lang.org/issues/18553

* Author: ibylich (Ilya Bylich)
* Status: Open
* Priority: Normal
* 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/

[ruby-core:108045] [Ruby master Bug#18553] Memory leak on compiling method call with kwargs

[jeremyevans0 (Jeremy Evans)]

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/

[ruby-core:114935] [Ruby master Bug#18553] Memory leak on compiling method call with kwargs

[peterzhu2118 (Peter Zhu) via ruby-core]

Issue #18553 has been updated by peterzhu2118 (Peter Zhu).

Status changed from Open to Closed

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

----------------------------------------
Bug #18553: Memory leak on compiling method call with kwargs
https://bugs.ruby-lang.org/issues/18553#change-104808

* Author: ibylich (Ilya Bylich)
* Status: Closed
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114940] Re: [Ruby master Bug#18553] Memory leak on compiling method call with kwargs

[Игорь Пятчиц via ruby-core]

[-- Attachment #1.1: Type: text/plain, Size: 2101 bytes --]

👍

пн, 2 окт. 2023 г. в 01:57, peterzhu2118 (Peter Zhu) via ruby-core <
ruby-core@ml.ruby-lang.org>:

> Issue #18553 has been updated by peterzhu2118 (Peter Zhu).
>
> Status changed from Open to Closed
>
> This was fixed in #19906, so I will close this issue.
>
> ----------------------------------------
> Bug #18553: Memory leak on compiling method call with kwargs
> https://bugs.ruby-lang.org/issues/18553#change-104808
>
> * Author: ibylich (Ilya Bylich)
> * Status: Closed
> * 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/
>  ______________________________________________
>  ruby-core mailing list -- ruby-core@ml.ruby-lang.org
>  To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
>  ruby-core info --
> https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
>

[-- Attachment #1.2: Type: text/html, Size: 3103 bytes --]

[-- Attachment #2: Type: text/plain, Size: 264 bytes --]

 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/