Skip to content
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

[9.1.7.0] Refinements of a superclass's singleton_class not being honored #4514

Closed
itspomf opened this Issue Feb 27, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@itspomf
Copy link

itspomf commented Feb 27, 2017

Scenario

A Refinement performed against the .singleton_class of an object's super-class is not being made active / available even when using the refinement in which it is declared and being within a scope wherein this refinement would be active.

IMPORTANT: This may be limited to JRuby's implementation of FFI::Struct within Java?

Environment

  • JRuby 9.1.7.0
  • Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +indy +jit
  • MS Windows 7 64-bit

Source Code used:

module Example
  refine FFI::Struct.singleton_class do
    def new( *args, **kwargs )
      return super( *args ) if kwargs.empty?
      obj = allocate()
      obj.send( :initialize, *args )
      kwargs.each {|k,v| obj[ k.to_sym ] = v rescue nil }
      obj
    end
  end
end

Trivial test code used:

class Thingus < FFI::Struct
  layout a: :int,
         b: :int
end

using Example
puts Thingus.new( a: 1, b: 2 )

Expected Behavior

Ruby 2.3.3p222 x64-mingw32 performs as follows:

#<Thingus:0x00000002e62bf0>

Actual Behavior

TypeError: wrong argument type Hash (expected Pointer or Buffer)
        from org/jruby/ext/ffi/Struct.java:141:in `initialize'
        from org/jruby/ext/ffi/Struct.java:167:in `initialize'
        from (irb):8:in `<module:Test>'
        from (irb):8:in `<eval>'
        from org/jruby/RubyKernel.java:1000:in `eval'
        from org/jruby/RubyKernel.java:1298:in `loop'
        from org/jruby/RubyKernel.java:1120:in `catch'
        from org/jruby/RubyKernel.java:1120:in `catch'
        from D:/JRuby/bin/jirb:13:in `<main>'

Affected lines:

Workaround:

This can, however, be corrected by explicitly declaring the singleton_class of the target to be affected, as demonstrated below:

module Thingummy
  refine Thingus.singleton_class do
    def new( *args, **kwargs )
      return super( *args ) if kwargs.empty?
      obj = allocate()
      obj.send( :initialize, *args )
      kwargs.each {|k,v| obj[ k.to_sym ] = v rescue nil }
      obj
    end
  end
end

# It will now work correctly here.
using Thingummy
puts Thingus.new( a: 1, b: 2 )

Output after workaround:

#<Thingus:0x217ed35e>

Possible Cause

This might be similar to Ruby 2.3 and lower's issue with applying refinements to methods which are indirectly acquired, such as BasicObject#method_missing() being unable to be refined, though how this determination is made is unknown to me.

That the Java source code is the root of the error and is always used unless an explicit refinement is given leads me to believe there is a possible connection. Below is an example of the exact same behavior on a native Ruby object where no such error is encountered.

module MathMixin
  refine Math.singleton_class do
    def min( a, b )
      a < b ? a : b
    end
  end
end

using MathMixin
Math.min( 1, -1 )  # => -1

Also of note is that standard Ruby has a different #inspect() output, which more clearly identifies the module as a refinement:

irb(main):012:0> module MathMixin
irb(main):013:1>   refine Math.singleton_class do
irb(main):014:2*     def min( a, b )
irb(main):015:3>       a < b ? a : b
irb(main):016:3>     end
irb(main):017:2>   end
irb(main):018:1> end
=> #<refinement:#<Class:Math>@MathMixin>

This is contrary to JRuby, which only shows:

irb(main):001:0> module MathMixin
irb(main):002:1>   refine Math.singleton_class do
irb(main):003:2*     def min( a, b )
irb(main):004:3>       a < b ? a : b
irb(main):005:3>     end
irb(main):006:2>   end
irb(main):007:1> end
=> #<Module:0x1d2adfbe>

Don't know if an issue was opened for this, but it doesn't show up in any of the open or closed issues I saw.

@headius

This comment has been minimized.

Copy link
Member

headius commented Mar 3, 2017

Still broken as of 9.1.8.0. We'll want to get this fixed up sooner rather than later, so targeting 9.2.

@itspomf

This comment has been minimized.

Copy link
Author

itspomf commented Jun 14, 2018

Did some more experimentation on this one with the 9.2 release. After looking through the trace, shown below, it looks like we're hitting the same general problem as before: the Hash of keyword arguments is being passed into ext/ffi/Struct.java when it shouldn't be.

Trace output

TypeError: wrong argument type Hash (expected Pointer or Buffer)
  initialize at org/jruby/ext/ffi/Struct.java:143
  initialize at org/jruby/ext/ffi/Struct.java:169
      <main> at vkdiag.rb:38

The Vulkan wrapper source code I'm using declares the following refinement, designed to provide an "in-line" means of declaring struct fields in a manner similar to C++. Debug lines have been added for demonstrative purposes.

Refinement code (lib/ctypes.rb)

# A refinement for the FFI::Struct class to provide better convenience in its use.
module Structs
    . . .

    refine FFI::Struct.singleton_class do
        . . .

        # Fancy initialization hack via keywords as members.
        def new( *args, **kwargs )
            puts "DEBUG:  #{self}.new( #{args.inspect}, #{kwargs.inspect} )" if $DEBUG
            return super( *args ) if kwargs.empty?
            puts "DEBUG:  Allocating new instance of #{self} ..." if $DEBUG
            obj = allocate()
            puts "DEBUG:  Setting members ..." if $DEBUG
            kwargs.each {|k,v| obj[ k.to_sym ] = v rescue nil }
            obj
        end
    end
end

Expectation

Now, based on the snippet above, we would expect to see something like the following:

DEBUG:  VkApplicationInfo.new( [], { :sType => :VK_STRUCTURE_TYPE_APPLICATION_INFO, :apiVersion => VK_API_VERSION_1_0, :pApplicationName =>  #<FFI::MemoryPointer 0x0000f32b> } )
DEBUG:  Allocating new instance of VkApplicationInfo ...
DEBUG:  Setting members ...

... and thereafter have a new instance of a VkApplicationInfo struct pointer.

The Reality

Unfortunately, the refinement code is completely bypassed and, despite no Ruby errors being raised when the lib/ctypes.rb file is required or when using Structs is called, the refinement against FFI::Struct's singleton class never applies.

This at first appears to be due to how Ruby silently discards refinements of indirectly inherited methods and similar, a behavior which seems to have been introduced around Ruby 2.3 or later (see: Ruby Feature #13129, regarding method_missing and respond_to_missing? in BasicObject)

Thus we get the TypeError above, due to Java taking over the creation completely.

However, my past investigation shows a very interesting disconnect: explicitly using the code below will resolve the issue.

module Structs
    refine VkApplicationInfo.singleton_class do
        . . .

It almost looks like we're getting a similar problem with dynamic and static rebinding as Matz described in the Ruby issue linked above. I would hazard to say that, because this is resolved with an explicit class declaration, JRuby 9.1.7.0 and later are not activating inherited refinements, contrary to Ruby 2.3 and later.

We can illustrate this by inspecting the code around the two lines referenced in JRuby 9.2.0.0's code, where the errors originate from. This largely confirms that the generic, inherited refinement is being bypassed and JRuby's Struct's initialize() is being called directly.

org/jruby/ext/ffi/Struct.java:169

    @JRubyMethod(name = "initialize", visibility = PRIVATE, rest = true)
    public IRubyObject initialize(ThreadContext context, IRubyObject[] args) {
        switch (args.length) {
            default: // > 1
                IRubyObject result = getMetaClass().callMethod(context, "layout", args[1] instanceof RubyArray
                        ? ((RubyArray) args[1]).toJavaArrayUnsafe()
                        : java.util.Arrays.copyOfRange(args, 1, args.length));
                if (!(result instanceof StructLayout)) {
                    throw context.runtime.newTypeError("Struct.layout did not return a FFI::StructLayout instance");
                }
                layout = (StructLayout) result;
            case 1: return initialize(context, args[0]);
            case 0: return initialize(context);
        }
    }

org/jruby/ext/ffi/Struct.java:143

    @JRubyMethod(name = "initialize", visibility = PRIVATE)
    public IRubyObject initialize(ThreadContext context, IRubyObject ptr) {
        
        if (!(ptr instanceof AbstractMemory)) {
            if (ptr.isNil()) {
                return initialize(context);
            }

            throw context.runtime.newTypeError("wrong argument type "
                    + ptr.getMetaClass().getName() + " (expected Pointer or Buffer)");
        }

        . . .
    }
@itspomf

This comment has been minimized.

Copy link
Author

itspomf commented Jun 14, 2018

tl;dr version of the above: JRuby 9.1.7.0 and later cannot resolve the static rebinding of a refinement if it modifies the singleton class of a classes' superclass.

In short:

IF class A's .singleton_class defines a method AND class B's singleton class inherits this method,
THEN a refinement against A's singleton class WILL NOT be visible in B's singleton class.

This is contrary to Ruby 2.3 and later.

Edit: Did a bit more digging, and came across RubyModule.addActivatedRefinement, which seems like it could be where this disconnect occurs.

It seems that this only handles superclasses. I'm still investigating how to infer refinements on a superclass, however.

A working demonstration follows. This fails in JRuby 9.2.0.0:

module Fooify
  refine Numeric do
    def foo()
      'foo!'
    end
  end
end

using Fooify

# let's create an Integer, which is a subclass of Numeric
10.foo

This raises a NoMethodError: undefined method 'cake' for 10:Integer on JRuby.

@headius

This comment has been minimized.

Copy link
Member

headius commented Feb 26, 2019

This appears to work after #5604 and #5627.

@headius headius closed this Mar 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.