The dangers of #undef_method, #instance_exec recalled for memleaking!

After reading this thread on ruby-talk, I reviewed my instance_exec implementation and found it totally unacceptable. I used #undef_method instead of #remove_method, what a fool I was! I'll first show why it can be a problem and then explain the causes; the issue is quite general, affecting a number of meta-hacks.

Take this script that just defines lots of methods to undefine them right away, measuring the VmSize as it goes*1

ITER = 10

def vm_size
  File.read("/proc/#{Process.pid}/status")[/^VmSize:\s+(\d+) kB/, 1].to_i
end

vm1 = vm_size
puts "I'm #{Process.pid}, using #{vm1} kB."
b = lambda{}
ITER.times do |i|
  (i*10000...(i+1)*10000).each do |i|
    name = "foo%6d" % i
    Object.module_eval{ define_method(name, &b); undef_method name }
  end
  GC.start
end
inc = vm_size - vm1
puts "undef_method"
puts "Increment: #{inc}, #{inc * 1024 / (ITER * 10000)} bytes per method."

Here's the output:

$ ruby undef_method.rb 
I'm 1343, using 3020 kB.
undef_method
Increment: 23092, 118 bytes per method.

The script is saying that each method definition is taking around 120 bytes, even when the method is removed right away.*2

This is not surprising: the memleak is caused by the symbols. But that's not all. I claimed that #undef_method was worse than #remove_method, and here's the proof: running the above script after substituting undef_method with remove_method yields this:

$ ruby remove_method.rb 
I'm 3170, using 3020 kB.
remove_method
Increment: 6932, 70 bytes per method.

The increment per method remains consistently at least 40+ bytes smaller than for #undef_method, so there's something besides symbol leaking.

How undef_method works

The difference lies in how remove_method and undef_method works. Before we come to that, a a few words about Ruby's implementation will help.

Internally, classes are represented by RClass structures which hold a m_tbl attribute pointing to a hash table that associates method bodies to their names (symbols). They also have an attribute named super that unsurprisingly points to the superclass (or the module/class-alike thing higher in the hierarchy, like the proxy classes created for mixins). When dispatching a method, Ruby searches the method tables for the appropriate method*3.

Back to #undef_method and #remove_method. This is what ri says about undef_method:

Prevents the current class from responding to calls to the named method.
Contrast this with +remove_method+, which deletes the method from the
particular class; Ruby will still search superclasses and mixed-in
modules for a possible receiver.

The wording is a bit unfortunate, but it says that after you do undef_method(:foo), objects will not repond to :foo even if it's defined in a superclass. Remembering how Ruby uses the method tables (m_tbl), it's clear that #undef_method works by registering a special "method" that essentially tells the interpreter "hey, stop looking for the method with that name, this is a missing method". This is expressed in C so:

 rb_add_method(klass, rb_intern(name), 0, NOEX_UNDEF);

On the other hand, all remove_method does is, well, remove the method from the m_tbl, without adding anything in exchange. Reading a bit further in ruby's sources (struct st_table_entry in st.c and NODE in node.h), it can be seen that this accounts for a difference of at least 40 bytes, the space needed to hold the special "this method is undefined" marker. That is, about the difference the VmSize-based tests indicated.

A bounded-space instance_exec

So it's clear by now that #remove_method is preferable in this case. However, this only brings the leak down to ~70 bytes per call, surely better than ~120 bytes but still too high. The only way to turn that into about 0 bytes is getting the work done without creating insane (one per call) amounts of symbols.

Here's yet another instance_eval implementation that satisfies that condition. Remember: it is thread-safe, reentrant, frozen-object-safe and now bounded-space too*4.

class Object
  module InstanceExecHelper; end
  include InstanceExecHelper
  def instance_exec(*args, &block)
    begin
      old_critical, Thread.critical = Thread.critical, true
      n = 0
      n += 1 while respond_to?(mname="__instance_exec#{n}")
      InstanceExecHelper.module_eval{ define_method(mname, &block) }
    ensure
      Thread.critical = old_critical
    end
    begin
      ret = send(mname, *args)
    ensure
      InstanceExecHelper.module_eval{ remove_method(mname) } rescue nil
    end
    ret
  end
end

Tests

class Dummy
  def f
    :dummy_value
  end
end

require 'test/unit'
class TestInstanceEvalWithArgs < Test::Unit::TestCase
  def test_instance_exec
    # Create a block that returns the value of an argument and a value
    # of a method call to +self+.  
    block = lambda { |a| [a, f] }

    assert_equal [:arg_value, :dummy_value], 
      Dummy.new.instance_exec(:arg_value, &block)
  end

  def test_instance_exec_with_frozen_obj
    block = lambda { |a| [a, f] }

    obj = Dummy.new
    obj.freeze
    assert_equal [:arg_value, :dummy_value],
      obj.instance_exec(:arg_value, &block)
  end

  def test_instance_exec_nested
    i = 0
    obj = Dummy.new
    block = lambda do |arg|
      [arg] + instance_exec(1){|a| [f, a] }
    end

    # the following assertion expanded by the xmp filter automagically from:
    # obj.instance_exec(:arg_value, &block) #=>
    assert_equal([:arg_value, :dummy_value, 1], obj.instance_exec(:arg_value, &block))
  end
end

#>> Loaded suite -
#>> Started
#>> .
#>> Finished in 0.000566 seconds.
#>> 
#>> 1 tests, 1 assertions, 0 failures, 0 errors
# >> Loaded suite -
# >> Started
# >> ...
# >> Finished in 0.00094 seconds.
# >> 
# >> 3 tests, 3 assertions, 0 failures, 0 errors

ActiveRecord - Jeff Lindsay (2006-09-29 (Fri) 01:32:35)

Love this, but when using with ActiveRecord I get some very weird side effects: http://rafb.net/paste/results/HoC17M79.html

Thread.critical being left 'true' - Lasse (2006-07-10 (Mon) 06:17:49)

I'm probably just showing my ignorance of Thread.critical but isn't the above implementation of instance_exec always leaving Thread.critical 'true'?

mfp 2006-07-10 (Mon) 09:00:41

yup, s/rescue/ensure/ of course

thx

tom 2006-07-26 (Wed) 14:12:03

Thanks for the instance_exec implementation, just what I needed!

mfp 2006-07-26 (Wed) 15:07:18

Happy to be of use :)