[ruby-core:114463] [Ruby master Bug#19436] Call Cache for singleton methods can lead to "memory leaks"

Issue #19436 has been updated by cedricm (C=E9dric Ma=EFon).





Hi,



Will this be backported to 3.2?

I'm currently seeing this in production on a large Rails app: when YJIT is =
enabled, after usual initial warmup, unicorn workers memory usage continue =
to grows linearly until reaching a significant amount of RAM -- at which po=
int they are either killed by a monitoring tool due to unreasonable memory =
usage (>650MB), or some cache gets flushed and a big chunk of memory gets r=
eleased (and then it goes back for another round).

Memory usage is perfectly stable without YJIT (~420MB).

I believe that this is linked to this bug.



Cheers



----------------------------------------

Bug #19436: Call Cache for singleton methods can lead to "memory leaks"

https://bugs.ruby-lang.org/issues/19436#change-104229



* Author: byroot (Jean Boussier)

* Status: Closed

* Priority: Normal

* Assignee: ko1 (Koichi Sasada)

* Target version: 3.3

* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN

----------------------------------------

Using "memory leaks" with quotes, because strictly speaking the memory isn'=
t leaked, but it can nonetheless lead to large memory overheads.



### Minimal Reproduction



```ruby

module Foo

  def bar

  end

end



def call_bar(obj)

  # Here the call cache we'll keep a ref on the method_entry

  # which then keep a ref on the singleton_class, making that

  # instance immortal until the method is called again with

  # another instance.



  # The reference chain is IMEMO(callcache) -> IMEMO(ment) -> ICLASS -> CLA=
SS(singleton) -> OBJECT

  obj.bar

end



obj =3D Object.new

obj.extend(Foo)



call_bar(obj)



id =3D obj.object_id

obj =3D nil



4.times { GC.start }

p ObjectSpace._id2ref(id)

```



### Explanation



Call caches keep a strong reference onto the "callable method entry" (CME),=
 which itself keeps a strong reference on the called object class

and in the cache of a singleton class, it keeps a strong reference onto the=
 `attached_object` (instance).



This means that any call site that calls a singleton method, will effective=
ly keep a strong reference onto the last receiver.

If the method is frequently called it's not too bad, but if it's infrequent=
ly called, it's effectively a (bounded) memory leak.

And if the `attached_object` is big, the wasted memory can be very substant=
ial.



### Practical Implications



Once relative common API impacted by this is [Rails' `extending` API](https=
://api.rubyonrails.org/classes/ActiveRecord/QueryMethods.html#method-i-exte=
nding). This API allow to extend a "query result set" with a module.

These query results set can sometimes be very big, especially since they ke=
ep references to the instantiated `ActiveRecord::Base` instances etc.



### Possible Solutions



#### Only keep a weak reference to the CME



The fairly "obvious" solution is to keep a weak reference to the CME, that'=
s what I explored in https://github.com/ruby/ruby/pull/7272, and it seems t=
o work.

However in debug mode It does fail on an assertion during compaction, but i=
t's isn't quite clear to me what the impact is.



Additionally, something that makes me think this would be the right solutio=
n, is that call caches already try to avoid marking the class:



```c

# vm_callinfo.h:275



struct rb_callcache {

    const VALUE flags;



    /* inline cache: key */

    const VALUE klass; // should not mark it because klass can not be free'd

                       // because of this marking. When klass is collected,

                       // cc will be cleared (cc->klass =3D 0) at vm_ccs_fr=
ee().

```



So it appears that the class being also marked through the CME is some kind=
 of oversight?



#### Don't cache based on some heuristics



If the above isn't possible or too complicated, an alternative would be to =
not cache CMEs found in singleton classes, except if it's the the singleton=
 class of a `Class` or `Module`.



It would make repeated calls to such methods slower, but the assumption is =
that it's unlikely that these CME would live very long.



#### Make `Class#attached_object` a weak reference



Alternatively we could make the `attached_object` a weak reference, which w=
ould drastically limit the amount of memory that may be leaked in such scen=
ario.



The downside is that `Class#attached_object` was very recently exposed in R=
uby 3.2.0, so it means changing its semantic a bit.





cc @peterzhu2118 @ko1







--=20

https://bugs.ruby-lang.org/

 ______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-c=
ore.ml.ruby-lang.org/