New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with Class.instance_methods after Module.prepend #4723

Closed
bmulvihill opened this Issue Jul 24, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@bmulvihill
Contributor

bmulvihill commented Jul 24, 2017

Hello - We are currently in the process of upgrading our Jruby application to Rails 5, and have discovered an issue with the axslx gem, which appears to stem from a Jruby bug. There is an issue open regarding this here:
randym/axlsx#533

This gem uses Array.instance_methods to delegate methods to an internal array. The gem works fine on the Rails 4-2 stable branch. I did some digging and discovered there is some strange behavior using prepend, which is used in Rails 5 on the Enumerable module. If the prepended module is included, and then prepended the including class will not show the modules methods in its instance_methods method.

Example from Rails:

In Rails 4-2 stable using Jruby 9.1.12.0:

 Array.instance_methods.include?(:each_with_index)
=> true

In Rails 5-1 stable using Jruby 9.1.12.0:

Array.instance_methods.include?(:each_with_index)
=> false

I think I reproduced the problem below outside of Rails. Let me know if you need anymore information.

Environment

Provide at least:

  • JRuby version: jruby 9.1.12.0

Expected Behavior

module A
def a
end
end

module B
def b
end
end

module C
def c
end
end

class D
include A
include B
end

D.instance_methods.include?(:b)
=> true

B.prepend C

D.instance_methods.include?(:b)
=> true

Actual Behavior

D.instance_methods.include?(:b)
=> true

B.prepend C

D.instance_methods.include?(:b)
=> false
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 18, 2017

Member

Definitely seems to be limited to prepends into modules.

[] ~/projects/jruby $ jruby -e "p Array.instance_methods.sort.include? :each_with_index"
true

[] ~/projects/jruby $ jruby -e "Array.prepend(Module.new); p Array.instance_methods.sort.include? :each_with_index"
true

[] ~/projects/jruby $ jruby -e "Enumerable.prepend(Module.new); p Array.instance_methods.sort.include? :each_with_index"
false
Member

headius commented Aug 18, 2017

Definitely seems to be limited to prepends into modules.

[] ~/projects/jruby $ jruby -e "p Array.instance_methods.sort.include? :each_with_index"
true

[] ~/projects/jruby $ jruby -e "Array.prepend(Module.new); p Array.instance_methods.sort.include? :each_with_index"
true

[] ~/projects/jruby $ jruby -e "Enumerable.prepend(Module.new); p Array.instance_methods.sort.include? :each_with_index"
false
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 18, 2017

Member

No luck so far in my exploration. I returned to MRI source to see how they walk the hierarchy, and as far as I can tell they don't do anything special for prepends on modules. The problem as I see it is that once prepended, Enumerable's methods move up to the prepend wrapper, and only the prepended module's methods get walked because we don't expect a hierarchy within modules.

However we're able to call those methods just fine, so method searching is doing the right thing.

Member

headius commented Aug 18, 2017

No luck so far in my exploration. I returned to MRI source to see how they walk the hierarchy, and as far as I can tell they don't do anything special for prepends on modules. The problem as I see it is that once prepended, Enumerable's methods move up to the prepend wrapper, and only the prepended module's methods get walked because we don't expect a hierarchy within modules.

However we're able to call those methods just fine, so method searching is doing the right thing.

headius added a commit that referenced this issue Aug 23, 2017

Include hierarchy of prepends when adding method names from mod.
Fixes #4723

Method searching for calls used overridden logic in the included
module wrapper, but we did not do the same logic when
traversing all methods to add them to a reflective method list
(such as instance_methods). This patch adds overridden logic in
the wrapper to walk the prepends just as in the call site case.

@headius headius closed this in 673b6e7 Aug 23, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 23, 2017

Member

Ok, so the hunch about method lookup paid off. We override method lookup for included modules to also walk the module's hierarchy of prepends, but had not done so for traversing all method names.

Member

headius commented Aug 23, 2017

Ok, so the hunch about method lookup paid off. We override method lookup for included modules to also walk the module's hierarchy of prepends, but had not done so for traversing all method names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment