Bug #17379
Refinement with modules redefinition bug
Description
Depending on the circumstance, a refinement can be modified even after being used:
def foo
[:base]
end
module M
def foo
super << :M
end
end
module Ext
refine Object do
include M
end
end
using Ext
p 'asd'.foo unless ENV['SKIP'] # => [:base, :M] (ok)
module M
def foo
super << :new_ref
end
end
p 'asd'.foo # => depends (not ok)
Running this gives:
$ ruby refinement.rb [:base, :M] [:base, :M] # => ok $ SKIP=t ruby refinement.rb [:base, :new_ref] # => should be [:base, :M]
Related issues
Updated by marcandre (Marc-Andre Lafortune) 3 months ago
- Subject changed from Refinement with modules redefinition issues to Refinement with modules redefinition bug
Updated by shugo (Shugo Maeda) 3 months ago
- Assignee changed from shugo (Shugo Maeda) to matz (Yukihiro Matsumoto)
It's an inline method cache issue and hard to solve without performance regression.
If the behavior is not acceptable as a limitation of Refinements, it may be better to prohibit module inclusion in Refinements.
Updated by shugo (Shugo Maeda) 3 months ago
shugo (Shugo Maeda) wrote in #note-2:
If the behavior is not acceptable as a limitation of Refinements, it may be better to prohibit module inclusion in Refinements.
I think there is no need to prohibit inclusion of frozen modules, at least for this issue.
Updated by shugo (Shugo Maeda) 3 months ago
- Assignee changed from matz (Yukihiro Matsumoto) to shugo (Shugo Maeda)
shugo (Shugo Maeda) wrote in #note-2:
It's an inline method cache issue and hard to solve without performance regression.
I might be wrong, and will investigate it further.
Updated by marcandre (Marc-Andre Lafortune) 3 months ago
Probably same bug, without using
, found by Daniel DeLorme:
class Foo
def foo
p :hello
end
end
module Code
def foo
p :A
end
end
module Extension
refine Foo do
prepend Code
end
end
Foo.new.foo unless ENV['SKIP'] # => :hello (ok)
Foo.prepend Code
Foo.new.foo # => depends (not ok)
gives:
$ ruby refinement.rb :hello :hello $ SKIP=t ruby refinement.rb :A
Updated by shugo (Shugo Maeda) 3 months ago
Updated by shugo (Shugo Maeda) 2 months ago
- Assignee changed from shugo (Shugo Maeda) to ko1 (Koichi Sasada)
It seems that callable method entry cache caused the problem.
The problem doesn't occur with the following patch:
diff --git a/vm_method.c b/vm_method.c index a0ccdb8a51..d3a3926780 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1023,7 +1023,7 @@ prepare_callable_method_entry(VALUE defined_class, ID id, const rb_method_entry_ mtbl = RCLASS_EXT(defined_class)->callable_m_tbl = rb_id_table_create(0); } cme = rb_method_entry_complement_defined_class(me, me->called_id, defined_class); - rb_id_table_insert(mtbl, id, (VALUE)cme); + // rb_id_table_insert(mtbl, id, (VALUE)cme); RB_OBJ_WRITTEN(defined_class, Qundef, (VALUE)cme); VM_ASSERT(callable_method_entry_p(cme)); } @@ -1122,7 +1122,8 @@ callable_method_entry(VALUE klass, ID mid, VALUE *defined_class_ptr) VM_ASSERT(RB_TYPE_P(klass, T_CLASS) || RB_TYPE_P(klass, T_ICLASS)); RB_VM_LOCK_ENTER(); { - cme = cached_callable_method_entry(klass, mid); + // cme = cached_callable_method_entry(klass, mid); + cme = NULL; if (cme) { if (defined_class_ptr != NULL) *defined_class_ptr = cme->defined_class; @@ -1139,7 +1140,7 @@ callable_method_entry(VALUE klass, ID mid, VALUE *defined_class_ptr) cme = negative_cme(mid); } - cache_callable_method_entry(klass, mid, cme); + // cache_callable_method_entry(klass, mid, cme); } } RB_VM_LOCK_LEAVE();
(The fix of prepare_callable_method_entry() is for Kernel#method)
ko1 (Koichi Sasada), is there any way to fix the problem without performance regression?
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
- Related to Bug #17429: Prohibit include/prepend in refinement modules added