Bug #1711: Marshal Failing to Round-Trip Certain Recurisve Data Structures

• Status: Rejected
• Assignee: -
• Target version: -
• ruby -v: ruby 1.9.2dev (2009-07-01 trunk 23924) [i686-linux]

[runpaint (Run Paint Run Run)]

=begin
I have attached a script that fails to round-trip a recursive data structure on 1.9, but succeeds on 1.8. IOW, it prints true on 1.8; false on 1.9. I haven't been able to reduce it further because I'm unfamiliar with Marshal.

 $ ruby86 -vw /tmp/marshal.rb 
 ruby 1.8.6 (2009-06-08 patchlevel 369) [i686-linux]
 true

 $ ruby87 -vw /tmp/marshal.rb 
 ruby 1.8.7 (2009-06-12 patchlevel 174) [i686-linux]
 true

 $ ruby8 -vw /tmp/marshal.rb 
 ruby 1.8.8dev (2009-06-28) [i686-linux]
 true

 $ ruby -vw /tmp/marshal.rb 
 ruby 1.9.2dev (2009-07-01 trunk 23924) [i686-linux]
 false

=end

[yugui (Yuki Sonoda)]

=begin
class UserDefined

class Nested
  def ==(other)
    other.kind_of? self.class
  end
end

attr_reader :a, :b

def initialize
  @a = 'stuff'
  @b = @a
end

def _dump(depth)
  Marshal.dump [@a, @b]
end

def self._load(data)
  a, b = Marshal.load data

  obj = allocate
  obj.instance_variable_set :@a, a
  obj.instance_variable_set :@b, b

  obj
end

def ==(other)
  self.class === other and
  @a == other.a and
  @b == other.b
end

end

class UserDefinedWithIvar
attr_reader :a, :b, :c

def initialize
  @a = 'stuff'
  @a.instance_variable_set :@foo, :UserDefinedWithIvar
  @b = 'more'
  @c = @b
end

def _dump(depth)
  Marshal.dump [@a, @b, @c]
end

def self._load(data)
  a, b, c = Marshal.load data

  obj = allocate
  obj.instance_variable_set :@a, a
  obj.instance_variable_set :@b, b
  obj.instance_variable_set :@c, c

  obj
end

def ==(other)
  self.class === other and
  @a == other.a and
  @b == other.b and
  @c == other.c and
  @a.instance_variable_get(:@foo) == other.a.instance_variable_get(:@foo)
end

end

arr = []
myproc = Proc.new { |o| arr << o }
o1 = UserDefined.new;
o2 = UserDefinedWithIvar.new
obj = [o1, o2, o1, o2]
Marshal.load Marshal.dump(obj), myproc
p arr == [o1, o2, obj]

=end

[matz (Yukihiro Matsumoto)]

=begin
Hi,

In message "Re: [ruby-core:24105] [Bug #1711] Marshal Failing to Round-Trip Certain Recurisve Data Structures"
on Thu, 2 Jul 2009 02:30:15 +0900, Run Paint Run Run [e-mail] writes:

|I have attached a script that fails to round-trip a recursive data structure on 1.9, but succeeds on 1.8. IOW, it prints true on 1.8; false on 1.9. I haven't been able to reduce it further because I'm unfamiliar with Marshal.

1.9 Marshal.load with proc is incompatible with 1.8, as following:

• 1.8 load ignores the value returned from proc, whereas 1.9 load
  replaces the object by the value from proc.

• 1.9 load calls proc for recursively visited data. 1.8 dump only
  calls proc once.

That is to allow rebuilding loading data using proc. For
the example in [ruby-core:24361], replacing

myproc = Proc.new { |o| arr << o }

by

myproc = Proc.new { |o| arr << o; o }

should work as expected. The future action would be either:

(a) the new 1.9 behavior has meaning. since virtually no one uses
Marshal.load with proc, it's OK to change it for a good reason.

(b) incompatibility is bad. should be reverted to the original 1.8
behavior.

(c) or probably add another optional (or keyword) argument to enable
the new behavior, keeping the old behavior by default.

Any opinion?

						matz.

=end

[drbrain (Eric Hodel)]

=begin
On Jul 27, 2009, at 06:14, Yukihiro Matsumoto wrote:

In message "Re: [ruby-core:24105] [Bug #1711] Marshal Failing to
Round-Trip Certain Recurisve Data Structures"
on Thu, 2 Jul 2009 02:30:15 +0900, Run Paint Run Run <[e-mail]

writes:

I have attached a script that fails to round-trip a recursive data
structure on 1.9, but succeeds on 1.8. IOW, it prints true on 1.8;
false on 1.9. I haven't been able to reduce it further because I'm
unfamiliar with Marshal.

1.9 Marshal.load with proc is incompatible with 1.8, as following:

• 1.8 load ignores the value returned from proc, whereas 1.9 load
  replaces the object by the value from proc.

• 1.9 load calls proc for recursively visited data. 1.8 dump only
  calls proc once.

That is to allow rebuilding loading data using proc. For
the example in [ruby-core:24361], replacing

myproc = Proc.new { |o| arr << o }

by

myproc = Proc.new { |o| arr << o; o }

should work as expected. The future action would be either:

(a) the new 1.9 behavior has meaning. since virtually no one uses
Marshal.load with proc, it's OK to change it for a good reason.

(b) incompatibility is bad. should be reverted to the original 1.8
behavior.

(c) or probably add another optional (or keyword) argument to enable
the new behavior, keeping the old behavior by default.

Any opinion?

I haven't seen Marshal.load with proc in the wild, so I prefer (a).

In the future, I could imagine using the new 1.9 behavior in RubyGems
for backwards compatibility.

=end

[runpaint (Run Paint Run Run)]

=begin

1.9 Marshal.load with proc is incompatible with 1.8, as following:

• 1.8 load ignores the value returned from proc, whereas 1.9 load
  replaces the object by the value from proc.

• 1.9 load calls proc for recursively visited data. 1.8 dump only
  calls proc once.

Thank you for the clarification, matz. I've updated the specs for the first point as I understand it and it seems reasonable. I kind of understand the second point, but am not sure of the implications.

The following example looks to illustrate the difference. 1.8 calls the proc for the first element of the Array, doesn't call it for the second because that's recursive, then calls it again for the entire Array.

$ ruby86 -e 'a=[1]; a << a; Marshal.load(Marshal.dump(a), proc {|e| p e})'
1
[1, [...]]

1.9, however, also calls the proc once for the recursive element, but I don't claim to understand exactly what form the argument takes. In this case it's the data structure containing the recursive element before the recursive element was appended. Is this what's intended? Would this help or hinder rebuilding Marshal'd data?

$ ruby -e 'a=[1]; a << a; Marshal.load(Marshal.dump(a), proc {|e| p e})'
1
[1]
[1, [...]]
=end

[wanabe (_ wanabe)]

=begin
This issue should be moved to Feature, shouldn't it?
=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Sun, 4 Apr 2010 08:44:45 +0900,
_ wanabe wrote in [ruby-core:29242]:

This issue should be moved to Feature, shouldn't it?

It's not what is called round-trip, therefore wrong thing is
the precondition.

I think this ticket would be rejected.

--
Nobu Nakada

=end

[mame (Yusuke Endoh)]

• Status changed from Open to Rejected

=begin
Hi,

In 1.9, proc is called for each node of object tree,
even if the node is recursive.

$ ruby -e 'a=[1]; a << a; Marshal.load(Marshal.dump(a), proc {|e| p e})'
1 # First element
[1] # Second element (recursive node; array itself)
[1, [...]] # Generated array

In 1.8, the recursive node is passed to proc just once.

$ ruby -e 'a=[1]; a << a; Marshal.load(Marshal.dump(a), proc {|e| p e})'
1 # First element
# Second element is not called because it is recursive
[1, [...]] # Generated array

According to matz, this is intended spec change.

There is no problem anywhere. So I close this ticket.

--
Yusuke Endoh [e-mail]
=end