eigenclass logo
MAIN  Index  Search  Changes  PageRank  Login

Functions accepting blocks in Ruby's C API make for tricky bugs

I ran into a tricky bug today. I was working on an extension that uses an event_hook and needed quite some time to figure this out:

require 'binding_n'
def e
  l = eval("local_variables", binding)
  puts "The array: #{l.inspect}"
  l.map do |str|
    puts "PROCESSING #{str.inspect}"
    [str, eval(str, binding)]
  end
end

Kernel.install_binding_n_hook
e
Kernel.remove_binding_n_hook

The block was evaluated twice, even though the array had but one element:

The array: ["l"]
PROCESSING "l"
PROCESSING nil
t.rb:7:in `eval': can't convert nil into String (TypeError)
        from t.rb:7:in `e'
        from t.rb:5:in `e'
        from t.rb:12

I finally tracked it down to this function, which was being executed on RUBY_EVENT_(C)_RETURN events:

static inline code_site_t *
callsite_stack_pop(callsite_stack_t *stack)
{
    if (stack->ptr == stack->start) {
        return stack->ptr;
    }
    rb_hash_delete(additional_gc_roots, (stack->ptr-1)->binding); 
    return --stack->ptr;
}

Can you spot the culprit?

Methods taking a block and implemented in C, at risk

If the key is not found, rb_hash_delete will yield it to the current block, if present. But which block is that? It's harder to see in C...

That would be the block passed to the current method, i.e. the one you rb_define_method()'d. Or, in this particular case, the method we were returning from (remember this was being executed in an event_hook).

This means that you have to be careful whenever you use a function from Ruby's C API that admits a block and you do not want one passed. The fix looks like this:

static VALUE
remove_gc_root(VALUE arg)
{
  rb_hash_delete(additional_gc_roots, arg);
}

static VALUE
dummy(VALUE arg1, VALUE arg2)
{
  return Qnil;
}

static inline code_site_t *
callsite_stack_pop(callsite_stack_t *stack)
{
    if (stack->ptr == stack->start) {
        return stack->ptr;
        //rb_raise(rb_eException, "empty stack");
    }
    /* this is what we want to do, but it's not safe because rb_hash_delete
     * will reuse the current block ... */
     /* rb_hash_delete(additional_gc_roots, (stack->ptr-1)->binding); */
    rb_iterate(remove_gc_root, (stack->ptr-1)->binding, dummy, Qnil);
    return --stack->ptr;
}

I think I've hit that - Daniel Berger (2006-08-18 (Fri) 15:03:49)

If I recall, I had to resort to rb_funcall once or twice. Maybe that was a different situation, though.

mfp 2006-08-19 (Sat) 08:11:19

I assumed rb_iterate with a dummy block would be faster, but I should benchmark this. Someday.


Last modified:2006/08/18 05:40:38
Keyword(s):[blog] [ruby] [frontpage] [extension] [block] [bug]
References: