From: eregontp@...
Date: 2020-07-25T10:32:52+00:00
Subject: [ruby-core:99331] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

Issue #17048 has been updated by Eregon (Benoit Daloze).


Should we rather design a good way to allow copying but yet not have to deal with uninitialized state?

Right now the only well-defined protocols for copying are
* `dup = allocate, copy @ivars, initialize_dup (which calls initialize_copy)`
* `clone = allocate, copy @ivars, initialize_clone (which calls initialize_copy), clone also copies extra state like frozen and the singleton class`

This means some classes have to support an "unintialized state" when otherwise they would not need to.
And in some cases it even means instances have to be mutable when they would otherwise not need to (e.g., MatchData, #16294).

So maybe we should make Module.allocate and #initialize_copy always raise, and override `dup` and `clone` for Module?
It's still unfortunate that this would mean duplicating the logic for dup/clone there.
So I think a better copying protocol is warranted here as it's not just an issue for Module.

Re @nobu's patch I don't like this ad-hoc condition which leaks implementation details into semantics.
How about having an `initialized` flag that's set by `#initialize` and `#initialize_copy` and checked in both of these methods if we want a quick fix?

----------------------------------------
Bug #17048: Calling initialize_copy on live modules leads to crashes
https://bugs.ruby-lang.org/issues/17048#change-86726

* Author: alanwu (Alan Wu)
* Status: Open
* Priority: Normal
* ruby -v: ruby 2.8.0dev (2020-07-23T14:44:25Z master 098e8c2873) [x86_64-linux]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------


Here's a repro script
```ruby
loop do
  m = Module.new do
    prepend Module.new
    def hello
    end
  end

  klass = Class.new { include m }
  m.send(:initialize_copy, Module.new)
  GC.start

  klass.new.hello rescue nil
end
```

Here's a script that shows that it has broken semantics even
when it happens to not crash.

```ruby
module A
end

class B
  include A
end

module C
  Const = :C
end

module D
  Const = :D
end

A.send(:initialize_copy, C)
p B::Const # :C, makes sense
A.send(:initialize_copy, D)
p B::Const # :D, makes sense
A.send(:initialize_copy, Module.new)
p (begin B::Const rescue NameError; 'NameError' end) # NameError, makes sense
A.send(:initialize_copy, C)
p B::Const # still NameErorr. Weird
```
This example shows that the problem exists [as far back as 2.0.0](https://wandbox.org/permlink/4dVDY9sNXJ803jh8).

I think the easiest way to fix this is to forbid calling `:initialize_copy`
on modules that have children. Another option is to try to decide on
the semantics of this. Though I don't think it's worth the effort as this
has been broken for a long time and people don't seem to to be using it.
Thoughts?




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>