Project

General

Profile

Actions

Bug #6538

closed

Mutability of Rational and Complex

Added by marcandre (Marc-Andre Lafortune) over 12 years ago. Updated over 12 years ago.

Status:
Rejected
Target version:
-
ruby -v:
r35875
Backport:
[ruby-core:45394]

Description

I hesitated to report this, but there is a "hole" in the immutability of Rational & Complex:

r = Rational(0) # Rationals are immutable
r.freeze        # but let's be certain and freeze it!
magic_trick(r)  # r is now changed:
r               # => (1/42)

The same thing occurs with Complex. I've left out the definition of magic_trick for anyone who wants to try and figure it out, but it's here: http://pastie.org/4016117

Is this worth fixing?


Files

0001-compatible-marshal-loader.patch (5.6 KB) 0001-compatible-marshal-loader.patch nobu (Nobuyoshi Nakada), 06/04/2012 10:24 PM

Related issues 1 (0 open1 closed)

Precedes Ruby master - Bug #6541: marshal_load discards frozen objectsClosednobu (Nobuyoshi Nakada)06/04/201206/04/2012Actions

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • Status changed from Open to Assigned
  • Assignee set to tadf (tadayoshi funaba)

Updated by tadf (tadayoshi funaba) over 12 years ago

  • Status changed from Assigned to Closed

anyway forbade it.
this is not a magic.
but this trick seems also effective to others.

r = Random.new
#=> #Random:0xb87a6d50
a = r.marshal_dump
#=> [...]

r.freeze
#=> #Random:0xb87a6d50
r.frozen?
#=> true

(0..10).map{r.rand}.to_a
#=> [0.31439277627295725, 0.22940914020883507, 0.1255127954343639, 0.22394641311330976, 0.5371394957199308, 0.6576211090277115, 0.7280552862872753, 0.4931093631805644, 0.6286785684826635, 0.6649446272706543, 0.32721416961292293]

r.marshal_load(a)
#=> #Random:0xb87a6d50

(0..10).map{r.rand}.to_a
#=> [0.31439277627295725, 0.22940914020883507, 0.1255127954343639, 0.22394641311330976, 0.5371394957199308, 0.6576211090277115, 0.7280552862872753, 0.4931093631805644, 0.6286785684826635, 0.6649446272706543, 0.32721416961292293]

Updated by marcandre (Marc-Andre Lafortune) over 12 years ago

  • Status changed from Closed to Open
  • Assignee deleted (tadf (tadayoshi funaba))

I see that this trick won't work if it's frozen after your commit, but the the freeze part was just more fun, sorry. It's the immutability of Rational and Complex that is at stake here.

r = Rational(0)
magic_trick(r)
r               # => (1/42)

This should not be possible, ideally, no?

Rationals and Complex are expected to be immutable. Also, not all logic is applied to marshal_load. There are all sorts of nasty tricks that one could play with this.

c = Complex(1)
r = Rational(0).marshal_load([1, c])
c.marshal_load([0,0])
r # => (1/(0+0i))

The fix would be ugly. Complex and Rational should not have marshal_load/dump methods. Using _load and _dump would break marshal format accross versions, so Marshal.{load|dump} would need to make special cases for Complex and Rational.

Updated by tadf (tadayoshi funaba) over 12 years ago

  • Assignee set to tadf (tadayoshi funaba)

class IM
def initialize(s) @s = s end
attr :s
end

i = IM.new(1)
i.s #=> 1
i.instance_eval{@s = 100}
i.s #=> 100

is this a mutable?

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

=begin
Marshal compatibilities for (({Complex})) and (({Rational})) are already broken,
since the time they were built in.
=end

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • Status changed from Open to Assigned

=begin
What about adding "uninitialized (just allocated) flag" and checking
if it is set in (({marshal_load})) method?
=end

Updated by tadf (tadayoshi funaba) over 12 years ago

i thought it first.
but it is cheap trick too (or nils filled instead).

i doubt about this issue.
really need?

Updated by marcandre (Marc-Andre Lafortune) over 12 years ago

Hi,

nobu (Nobuyoshi Nakada) wrote:

Marshal compatibilities for (({Complex})) and (({Rational})) are already broken,
since the time they were built in.

I didn't know. Changing from #marshal_load to .load would break marshalling between 1.9 and 2.0 though. Unless marshal changes to handle both cases similarly? I mean, why are there two different codes for marshalling done via #marshal_dump vs _dump? It could be the same, if ._load exists, it is called, otherwise the object is constructed and #marshal_load is called?

tadf (tadayoshi funaba) wrote:

i doubt about this issue.
really need?

I have doubts too, which is why I asked "Is this worth fixing?"

Updated by tadf (tadayoshi funaba) over 12 years ago

  • Status changed from Assigned to Rejected

ok, it's unnecessary.

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

An alternative way is "marshal format compatibility layer".

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0