Bug #12353: Regression with Marshal.dump on ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]
• Backport: 2.1: DONTNEED, 2.2: DONTNEED, 2.3: REQUIRED

[jeffreyc (Jeff C)]

Attempting to call Marshal.dump on an ActiveSupport::CoreExtensions::Numeric::Time (e.g., 1.day) triggers a NoMethodError: undefined method `marshal_dump' for 86400:Fixnum exception in both Ruby 2.3.0 and 2.3.1 on OS X (10.11.4) and in Travis CI with ActiveSupport 4.1.15. This works correctly in previous versions, including 2.2.5.

To reproduce:

% rbenv install 2.3.1
...
% rbenv global 2.3.1
% gem install activesupport -v 4.1.15
...
% ruby --version    
ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]
% irb
irb(main):001:0> require 'active_support/core_ext/numeric/time'
=> true
irb(main):002:0> Marshal.dump(1.day)
NoMethodError: undefined method `marshal_dump' for 86400:Fixnum
	from ~/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-4.1.15/lib/active_support/duration.rb:115:in `method_missing'
	from (irb):2:in `dump'
	from (irb):2
	from ~/.rbenv/versions/2.3.1/bin/irb:11:in `<main>'

In 2.2.5:

% rbenv install 2.2.5
...
% rbenv global 2.2.5
% gem install activesupport -v 4.1.15
...
% ruby --version
ruby 2.2.5p319 (2016-04-26 revision 54774) [x86_64-darwin15]
% irb                                
irb(main):001:0> require 'active_support/core_ext/numeric/time'
=> true
irb(main):002:0> Marshal.dump(1.day)
=> "\x04\bo:\x1CActiveSupport::Duration\a:\v@valuei\x03\x80Q\x01:\v@parts[\x06[\a:\tdaysi\x06"

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Feedback

I can't reproduce it.

$ gem2.3 install --user --no-doc activesupport
Fetching: i18n-0.7.0.gem (100%)
Successfully installed i18n-0.7.0
Fetching: activesupport-4.2.6.gem (100%)
Successfully installed activesupport-4.2.6
2 gems installed

$ ruby2.3 -v -ractive_support/core_ext/numeric/time -e 'p Marshal.dump(1.day)'
ruby 2.3.1p112 (2016-04-26 revision 54768) [universal.x86_64-darwin15]
"\x04\bo:\x1CActiveSupport::Duration\a:\v@valuei\x03\x80Q\x01:\v@parts[\x06[\a:\tdaysi\x06"

[jeffreyc (Jeff C)]

Nobuyoshi Nakada wrote:

I can't reproduce it.

Successfully installed activesupport-4.2.6

The regression appears to be specific to ActiveSupport 4.1.15. If I try ActiveSupport 4.2.6, Marshal.dump behaves as expected.

[bakineggs (Dan Barry)]

Using Ruby 2.3, Marshal.dump(1.day) works with ActiveSupport 4.2.6 but not 4.1.15 because ActiveSupport::Duration inherits from BasicObject in 4.1.15 and Object in 4.2.6.

The issue is that as of Ruby 2.3, Marshal.dump no longer works with objects that inherit from BasicObject and do not directly define a respond_to? method. This is because vm_respond_to() in vm_method.c returns true if it can not find a method table entry for the respond_to? method, so w_object() in marshal.c tries to call marshal_dump. In Ruby 2.2 and prior, rb_obj_respond_to() would just call respond_to? instead of checking for a method table entry, so method_missing could receive the respond_to?.

Here's a simple example:

$ irb
2.3.1 :001 > class Wrapper < BasicObject
2.3.1 :002?>   def initialize wrapped_object
2.3.1 :003?>     @wrapped_object = wrapped_object
2.3.1 :004?>   end
2.3.1 :005?>   def method_missing *args, &block
2.3.1 :006?>     @wrapped_object.public_send *args, &block
2.3.1 :007?>   end
2.3.1 :008?> end
 => :method_missing 
2.3.1 :009 > wrapped_string = Wrapper.new "foo"
 => "foo" 
2.3.1 :010 > wrapped_string.respond_to? :marshal_dump
 => false 
2.3.1 :011 > Marshal.dump wrapped_string
NoMethodError: undefined method `marshal_dump' for "foo":String
	from (irb):6:in `public_send'
	from (irb):6:in `method_missing'
	from (irb):11:in `dump'
	from (irb):11
	from /Users/dan/.rvm/rubies/ruby-2.3.1/bin/irb:11:in `<main>'
2.3.1 :012 > class Wrapper
2.3.1 :013?>   def respond_to? *args, &block
2.3.1 :014?>     super
2.3.1 :015?>   end
2.3.1 :016?> end
 => :respond_to? 
2.3.1 :017 > wrapped_string.respond_to? :marshal_dump
 => false 
2.3.1 :018 > Marshal.dump wrapped_string
 => "\x04\bo:\fWrapper\x06:\x14@wrapped_objectI\"\bfoo\x06:\x06ET"

In Ruby 2.2 and earlier, Marshal.dump(wrapped_string) would work without needing to define a respond_to? method on Wrapper.

It's bad practice to define a method_missing without a corresponding respond_to_missing?, but even with a respond_to_missing? method defined, we still need a respond_to? in order for Marshal.dump to work:

2.3.1 :001 > class Wrapper < BasicObject
2.3.1 :002?>   def initialize wrapped_object
2.3.1 :003?>     @wrapped_object = wrapped_object
2.3.1 :004?>   end
2.3.1 :005?>   def method_missing *args, &block
2.3.1 :006?>     @wrapped_object.public_send *args, &block
2.3.1 :007?>   end
2.3.1 :008?>   def respond_to_missing? method_name, include_private = false
2.3.1 :009?>     @wrapped_object.respond_to? method_name, include_private
2.3.1 :010?>   end
2.3.1 :011?> end
 => :respond_to_missing? 
2.3.1 :012 > wrapped_string = Wrapper.new "foo"
 => "foo" 
2.3.1 :013 > wrapped_string.respond_to? :marshal_dump
 => false 
2.3.1 :014 > Marshal.dump wrapped_string
NoMethodError: undefined method `marshal_dump' for "foo":String
	from (irb):6:in `public_send'
	from (irb):6:in `method_missing'
	from (irb):14:in `dump'
	from (irb):14
	from /Users/dan/.rvm/rubies/ruby-2.3.1/bin/irb:11:in `<main>'

One way to achieve the expected behavior would be to change vm_respond_to() so that it calls method_missing(:respond_to?, priv) when it can't find a method table entry for respond_to? instead of just returning true.

I'd be happy to make this change if someone on the core team can let me know if my suggestion is the right way to handle this or if there's a different way things should be changed.

[nobu (Nobuyoshi Nakada)]

• Status changed from Feedback to Closed

Applied in changeset r55500.

---

No respond_to? as if the default definition

• vm_method.c (vm_respond_to): try method_missing if respond_to?
  is undefined, as if it is the default definition.
  [ruby-core:75377] [Bug #12353]

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: DONTNEED, 2.2: DONTNEED, 2.3: REQUIRED

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.1: DONTNEED, 2.2: DONTNEED, 2.3: REQUIRED to 2.1: DONTNEED, 2.2: DONTNEED, 2.3: DONE

ruby_2_3 r55869 merged revision(s) 54142,55500.

[knu (Akinori MUSHA)]

• Status changed from Closed to Open

I found this backport broke ActiveSupport::Duration of ActiveSupport 4.1.x:

% ruby -e '
gem "activesupport", "4.1.16"
require "active_support/time"
10.days'
/PREFIX/lib/ruby/gems/2.3.0/gems/activesupport-4.1.16/lib/active_support/core_ext/numeric/time.rb:50:in `*': ActiveSupport::Duration can't be coerced into Fixnum (TypeError)
        from /PREFIX/lib/ruby/gems/2.3.0/gems/activesupport-4.1.16/lib/active_support/core_ext/numeric/time.rb:50:in `days'
        from -e:4:in `<main>'

This could be fixed by defining respond_to_missing? as follows:

class ActiveSupport::Duration
  def respond_to_missing?(name, private = false)
    value.respond_to?(name, false)
  end
end

However, some gems such as Devise call the time unit methods while they are required, and it is not always easy for a user application to monkey-patch this if it uses Bundler to require all gems at once.

Now, it has already been announced that Rails 4.1 is going to see EOL in no distant future, but considering Rails 4.1.16 was released pretty recently, I think we should probably cooperate with the Rails team and make sure the upcoming 2.3.2 release will not break existing Rails 4.1 applications without prior notice.

[nagachika (Tomoyuki Chikanaga)]

r55500 breaks 1.days, while it fixed Marshal.dump with object which respond to marshal_dump with method_missing.
Even though it's obviously a bug-fix, introduce more critical issue as for activesupport-4.1.x.
I think I should revert r55869 for a while.

[knu (Akinori MUSHA)]

So, can we revert the backport?

This issue spotted a feature bug and it got fixed in trunk, but the original problem (Marshall.dump(1.day) does not work with ruby 2.3+ and activesupport 4.1.x) was not solved by the fix, causing a more serious problem in terms of backward compatibility.

As was confirmed it works with activesupport 4.2+, so I think it will be fine if Ruby 2.4 and later do not run Rails 4.1, since it's almost certain that the 2.4 series will start off after 4.1 effectively reaches its EOL.

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Open to Closed

Applied in changeset ruby_2_3|r56022.

---

• vm_method.c: revert r55869. it breaks Integer#days with
  ActiveSupport-4.1.x. [ruby-core:76949] [Bug #12353]

• test/ruby/test_marshal.rb: ditto.

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Closed to Open
• Backport changed from 2.1: DONTNEED, 2.2: DONTNEED, 2.3: DONE to 2.1: DONTNEED, 2.2: DONTNEED, 2.3: REQUIRED

I reverted r55869 partially at r56022.
And I keep opend this ticket because there's still problem with activesupport 4.1.x.

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Closed