[ruby-core:99311] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[ruby-core:99311] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been reported by alanwu (Alan Wu).

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

* 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/ASonJzNTUvxMdP8A).

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/

[ruby-core:99312] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[merch-redmine]

Issue #17048 has been updated by jeremyevans0 (Jeremy Evans).


Removing `Module#initialize_copy` would probably require changes to `Module#{dup,clone}`.  Maybe we can set a flag in  `Module#initialize_copy` such that calling the method raises if called again on the same module (not sure if that is what you meant)?

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

* 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/

[ruby-core:99313] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been updated by alanwu (Alan Wu).


I'm not proposing that we remove `initialize_copy`, but to make it raise when the receiver has children. So this is what it would look like:

```ruby
module A
end

A.send(:initialize_copy, Module.new) # fine, no one inherits from A

class B
  include A
  A.send(:initialize_copy, Module.new) # raise, since A now has one child
end
```
Admittedly this proposed behavior is designed to sidestep the problem that the current implementation has.
Though maybe it's fine since this is a seldom used private method and other Ruby implementations don't have to follow this behavior?

Now that you've mentioned it, removing the method and moving the logic into `Module#{dup,clone}` feels like the cleanest solution. It's more likely to be a breaking change though.

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

* 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/

[ruby-core:99314] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[merch-redmine]

Issue #17048 has been updated by jeremyevans0 (Jeremy Evans).


I would recommend to have `initialize_copy` always raise instead of only raising if the module has children.  It's more consistent, and there is no reason anyone should be calling `initialize_copy` more than once.  This shouldn't be considered a breaking change, as `initialize_copy` is a private method.  I think it is better than moving the logic to `dup`/`clone`, just in case a Module subclass is overridding `initialize_copy` and calling super.

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

* 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/

[ruby-core:99317] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been updated by alanwu (Alan Wu).


In principal I agree that allowing initialization only once is a good way to go, but the way `Module.allocate` is currently setup makes implementing this a bit complicated. At the moment there is not really a pre-init state for modules and the result from `Module.allocate` is the same as `Module.new`. If we want to do this we would have to implement a pre-init state for modules, and make sure that all the operations on modules (adding methods, constants, etc) are aware of this state so they can do the initialization in case they receive a pre-init module.

It's doable, but I don't know if the extra consistency is worth the added complexity. It would slow things down a small bit for operations like method addition and has the potential to introduce crashes if we miss places where we need the initialization check.

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

* 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/

[ruby-core:99321] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[nobu]

Issue #17048 has been updated by nobu (Nobuyoshi Nakada).


I agree with @alanwu, that it won't be worth.

```diff
diff --git c/class.c i/class.c
index 6835d2d7289..f7a56601634 100644
--- c/class.c
+++ i/class.c
@@ -354,6 +354,13 @@ static void ensure_origin(VALUE klass);
 VALUE
 rb_mod_init_copy(VALUE clone, VALUE orig)
 {
+    if (RCLASS_EXT(clone)->subclasses ||
+        RCLASS_EXT(clone)->parent_subclasses ||
+        RCLASS_EXT(clone)->module_subclasses) {
+        rb_raise(rb_eTypeError, "cannot replace %s in use",
+                 (RB_TYPE_P(clone, T_MODULE) ? "module" : "class"));
+    }
+
     /* cloned flag is refer at constant inline cache
      * see vm_get_const_key_cref() in vm_insnhelper.c
      */
diff --git c/test/ruby/test_module.rb i/test/ruby/test_module.rb
index d2da384cbd1..8d986f13413 100644
--- c/test/ruby/test_module.rb
+++ i/test/ruby/test_module.rb
@@ -435,6 +435,12 @@
     assert_empty(m.constants, bug9813)
   end
 
+  def test_initialize_copy_in_use
+    m = Module.new
+    Class.new {include m}
+    assert_raise(TypeError) {m.send(:initialize_copy, Module.new)}
+  end
+
   def test_dup
     OtherSetup.call
 
```

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

* 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/

[ruby-core:99331] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[eregontp]

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/

[ruby-core:99336] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been updated by alanwu (Alan Wu).


> 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?

That doesn't work because you can trigger the bug without ever calling `initialize` on the module:

```ruby
m = Module.allocate
m.prepend(Module.allocate)
m.define_method(:hello) {}
klass = Class.new { include m }
m.send(:initialize_copy, Module.new)
GC.start
klass.new.hello rescue nil
# you may need to run this multiple times to get to to crash
```

If we want something like that we would have to implement an uninitialized state.




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

* 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/

[ruby-core:99343] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[nobu]

Issue #17048 has been updated by nobu (Nobuyoshi Nakada).


Tried it.
https://github.com/nobu/ruby/tree/uninitialized-module

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

* 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/

[ruby-core:99349] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[eregontp]

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


nobu (Nobuyoshi Nakada) wrote in #note-10:
> Tried it.
> https://github.com/nobu/ruby/tree/uninitialized-module

Right, removing `Module.allocate` is one way since `dup`/`clone` uses the alloc function directly (and does not call `allocate`).

I think calling `Module.allocate` in user code makes no sense, so that approach seems a good way to me.

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

* 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/

[ruby-core:99379] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been updated by alanwu (Alan Wu).


Thank you the code, Nobu! I think with your branch we could even keep `.allocate`, though people wouldn't be able to do much with it.
As long as no one is able to call `initialize_copy` after children (iclasses) exist, it's fine.
I think I was wrong about the number of places we would have to plug to implement an uninitialized state that resolves the issue.
Only the places that make new iclasses need to check for the uninitilaized state, so jsut `prepend`, `include` and maybe refinements.

Side note about the branch (57c7f9b), it's possible to get access to an uninitialized module in Ruby land by subclassing from `Module`:
```ruby
class Sub < Module
  def initialize_copy(other)
    p ancestors
  end
end

Sub.new.dup # [#<Sub:0x00007fa4ec015b10>, BasicObject]
```
It doesn't cause anything bad to happen AFAICT. I just found it interesting that the branch adds a normally impossible-to-construct module.
Maybe it's a positive because it makes Ruby more weird :D


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

* 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/

[ruby-core:99567] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[ko1]

Issue #17048 has been updated by ko1 (Koichi Sasada).


sorry I didn't check all threads, but nobu's patch can close it?

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

* 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/

[ruby-core:99715] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[XrXr]

Issue #17048 has been updated by alanwu (Alan Wu).


Yes, nobu's patch fixes the crash. It is technically a breaking change though, so maybe it needs approval from Matz?
Side note, the bug still exists on master as of [today](https://wandbox.org/permlink/afSmo1UbL9wnIBmq).

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

* 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/

[ruby-core:104475] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[merch-redmine]

Issue #17048 has been updated by jeremyevans0 (Jeremy Evans).


This example in the original post has been fixed recently in the master branch.  I bisected the fixing commit to commit:3dd3ea092acead6179033f2c95525ffc5b8bb6ff.  However, as that changes constant lookup order, I'm guessing we aren't going to want to backport it.

Ruby still allows calling `Module.allocate` and calling `Module#initialize_copy` on modules with subclasses.  And the example in #note-9 still can cause an occasional hang in my environment.  So I'm going to leave this open.

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

* 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/

[ruby-core:105288] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[nobu (Nobuyoshi Nakada)]

Issue #17048 has been updated by nobu (Nobuyoshi Nakada).


As far as I tested it with `master` slightly, it didn't reproduce.
Does it still happen, and how occational?

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

* 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/

[ruby-core:105312] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[jeremyevans0 (Jeremy Evans)]

Issue #17048 has been updated by jeremyevans0 (Jeremy Evans).


nobu (Nobuyoshi Nakada) wrote in #note-16:
> As far as I tested it with `master` slightly, it didn't reproduce.
> Does it still happen, and how occational?

It still happens in my environment.  I tested with 1000 iterations, and it hung on the 14th iteration:

```
$ for x in `jot 1000`; do echo -n .; ./ruby 17048-note9.rb; done
..............
```


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

* 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/

[ruby-core:105314] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes

[alanwu (Alan Wu)]

Issue #17048 has been updated by alanwu (Alan Wu).


Here a repro that that always crashes for me on cbbda3e:

```ruby
mod = Module.new { define_method(:foo) {:first} }
klass = Class.new { include mod }
instance = klass.new
p instance.foo
new_mod = Module.new { define_method(:foo) { :second } }
mod.send(:initialize_copy, new_mod)
4.times { GC.start }
p instance.foo # [BUG] unreachable
```

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

* 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/