Module method defined on metaclass in Java ext has wrong owner #3463

Closed
headius opened this Issue Nov 16, 2015 · 2 comments

Comments

Projects
None yet
1 participant
@headius
Member

headius commented Nov 16, 2015

Only affects 1.7. Reproduction can be found at https://github.com/samphippen/playground.git under call_origin_with_module. bundle install, then bundle exec ruby explain_the_bug.rb.

JRuby 9k and MRI:

$ bundle exec ruby explain_the_bug.rb 
uri:classloader:/jruby/kernel/kernel.rb:17: warning: unsupported exec option: close_others
Pure module singleton class owner #<Class:#<Module:0x77847718>>
Pure module singleton class owner equals singleton class? true
jrjackson singleton class owner #<Class:JrJackson::Raw>
jrjackson singleton class owner equals singleton class? true

JRuby 1.7.x:

$ bundle exec ruby explain_the_bug.rb 
Pure module singleton class owner #<Class:#<Module:0x795cd85e>>
Pure module singleton class owner equals singleton class? true
jrjackson singleton class owner JrJackson::Raw
jrjackson singleton class owner equals singleton class? false

Cause of rspec/rspec-mocks#964. Thanks @samphippen for the repro case.

I suspect this is a problem of divergent logic in the reflective JRubyMethod annotation binding. The jrjackson gem has a JRuby ext component, which would use the reflective logic. I could not reproduce with exts shipped with JRuby, and they all use our generated "Populator" code. I also fixed a number of discrepancies between the generated populators and the reflective populators for JRuby 9k while looking into pure-invokedynamic method objects. Somewhere in there is a fix we need to backport to 1.7.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 20, 2015

Member

Today.

Member

headius commented Nov 20, 2015

Today.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 20, 2015

Member

Looks like this is a one liner I fixed in 9k.

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
index d32e419..e59c53f 100644
--- a/core/src/main/java/org/jruby/RubyModule.java
+++ b/core/src/main/java/org/jruby/RubyModule.java
@@ -3734,6 +3734,7 @@ public class RubyModule extends RubyObject {
                 singletonClass = module.getSingletonClass();
                 // module/singleton methods are all defined public
                 DynamicMethod moduleMethod = dynamicMethod.dup();
+                moduleMethod.setImplementationClass(singletonClass);
                 moduleMethod.setVisibility(PUBLIC);

                 if (jrubyMethod.name().length == 0) {
Member

headius commented Nov 20, 2015

Looks like this is a one liner I fixed in 9k.

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
index d32e419..e59c53f 100644
--- a/core/src/main/java/org/jruby/RubyModule.java
+++ b/core/src/main/java/org/jruby/RubyModule.java
@@ -3734,6 +3734,7 @@ public class RubyModule extends RubyObject {
                 singletonClass = module.getSingletonClass();
                 // module/singleton methods are all defined public
                 DynamicMethod moduleMethod = dynamicMethod.dup();
+                moduleMethod.setImplementationClass(singletonClass);
                 moduleMethod.setVisibility(PUBLIC);

                 if (jrubyMethod.name().length == 0) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment