Rubinus: Unable to clone/dup String class (TypeError)

[dblock]

See dblock/grape-on-rack@de6c002.

This needs a repro + spec, but I think it's as simple as having a class in the parameters of an API, Grape mount will attempt to dup the whole thing and Rubinius refuses to dup classes.

[rnubel]

Hello! I did some research into this, and came up with a fix (#1038).

The root cause is that the duplicable? method (added by ActiveSupport and/or the backport in Grape) on String returns true, but in Rubinius, the String class actually can't be duped.

# ruby-2.1.5
irb(main):002:0> String.duplicable?
=> true
irb(main):003:0> { :foo => String }.deep_dup
=> {:foo=>#<Class:0xa61ec70>}

# rbx-2.5.5
irb(main):002:0> String.duplicable?
=> true
irb(main):003:0> { :foo => String }.deep_dup
TypeError: Unable to clone/dup String class
        from kernel/common/string.rb:39:in `dup (clone)'
        from /home/robert/.gem/rbx/2.1.0/gems/activesupport-4.2.2/lib/active_support/core_ext/object/deep_dup.rb:14:in `deep_dup'
        from /home/robert/.gem/rbx/2.1.0/gems/activesupport-4.2.2/lib/active_support/core_ext/object/deep_dup.rb:43:in `deep_dup'
        from kernel/common/enumerable.rb:83:in `each_with_object'
        from kernel/common/hash.rb:342:in `each'
        from kernel/common/enumerable.rb:81:in `each_with_object'
        from /home/robert/.gem/rbx/2.1.0/gems/activesupport-4.2.2/lib/active_support/core_ext/object/deep_dup.rb:42:in `deep_dup'
        from (irb):3

This affects InheritableValues' usage of deep_dup, so it blows up whenever a hash including a String-class key or value is duped (e.g., when you have an endpoint with a parameter that you describe with the String-class type).

So, if we just change String::duplicable? to return false, deep_dup will no longer try to duplicate it and this issue will be fixed. That's kind of hacky, but I doubt that modifying Rubinius' behavior here is an option (though I could run it by @brixen).

[brixen]

Thanks for the ping, @rnubel. I have not investigated this beyond reading this issue but a couple things come to mind immediately:

1. @evanphx unfortunately didn't leave a commit note explaining why he disabled cloning the String class and my memory of this is extremely vague, so I don't know offhand what the issue was; and
2. It's not clear to me why anyone would want to dup String itself, or any core Ruby class for that matter. To me, it sounds like a recipe for disaster in some distant corner of code and frankly, allowing it seems like sloppy programming to me. I'm very curious to hear the rationale.

As this IRB session shows, creating an instance of a clone of String would likely quack like a String, but wouldn't be a String. More importantly, I'm pretty sure (but have not written the C code to verify) that MRI's numerous internal checks on T_STRING would fail, with curious and obscure results.

$ irb
irb(main):001:0> klass = String.clone
=> #<Class:0x007ff2340509f0>
irb(main):002:0> s = klass.new
=> ""
irb(main):003:0> s.class
=> #<Class:0x007ff2340509f0>
irb(main):004:0> s.kind_of? String
=> false

[brixen]

To further clarify what I'm trying to illustrate with that IRB session, an instance of the clone of String is not a kind_of? String, like a subclass of String would be. There is no utility that I can see in this usage. So, I'm most curious why there is perceived utility in making a class that is a String in every behavioral way but is not kind_of? String, like a String itself, or a subclass of String, would be.

[rnubel]

@brixen: Thanks for the input! Definitely agreed. Dup-ing classes seems like a case of Ruby making it easy to shoot yourself in the foot. Even though Rubinius' behavior is inconsistent (String can't be duped, but other core classes can?), I can't say I think altering Rubinius to remove that TypeError is a good idea.

[dblock]

This is closed via #1048