[ruby-core:116666] [Ruby master Bug#20255] Embedded arrays aren't moved correctly across ractors
Issue #20255 has been reported by luke-gru (Luke Gruber). ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255 * Author: luke-gru (Luke Gruber) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). I sent a PR here: https://github.com/ruby/ruby/pull/9918 ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-106678 * Author: luke-gru (Luke Gruber) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by ko1 (Koichi Sasada). Assignee set to ko1 (Koichi Sasada) ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-106736 * Author: luke-gru (Luke Gruber) * Status: Open * Priority: Normal * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). Hi Jean, thanks for taking care of this issue. Your change makes sense to me but I have a small concern regarding the potential call to `initialize_clone`. When allowing user-defined hooks like this in Ractor move logic, the user can add a non-shareable object. For example: ```ruby a = [1, 2, 3] NON_SHAREABLE = Object.new p "outside: #{NON_SHAREABLE}" def a.initialize_clone(other) super instance_variable_set("@non_shareable", NON_SHAREABLE) end r = Ractor.new do obj = Ractor.receive p "inside: #{obj.instance_variable_get("@non_shareable")}" end r.send(a, move: true) r.take ``` ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112504 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). @luke-gru indeed. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112505 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). This change seems to have broken this case also: ```ruby a = Object.new NON_SHAREABLE = Object.new p "outside: #{NON_SHAREABLE}" a.instance_variable_set("@non_shareable", NON_SHAREABLE) r = Ractor.new do obj = Ractor.receive p "inside: #{obj}" p "inside: #{obj.instance_variable_get("@non_shareable")}" # we get an error here now end r.send(a, move: true) r.take ``` ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112506 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). For context, even though I didn't precisely predict these two cases, I was expecting `initialize_cone` to be problematic. In the `move: true` case we certainly shouldn't invoke it. But also, this patch isn't really meant as a final solution. Longer term I'd like to rely on the GC compaction routine to do the moving, but short term fixing this memory corruption issue, even in a non-ideal way, allow to unblock further efforts. I'll see about cloning without invoking the callbacks as another short term fix. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112507 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). Also I don't quite get why this is an issue with `move: true` but isn't when doing a copy. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112508 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). Yes, copy as well is a problem. The best solution is to not allow these callbacks, because even if they run and you try to traverse the graph again to error or copy/move it, the callback could have spawned a thread that sets an unshareable on the object and then we're back to square one. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112509 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). https://github.com/ruby/ruby/pull/13023 ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112510 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier).
The best solution is to not allow these callbacks,
So it's not actually possible, because many types rely on it. e.g: ```ruby class Array def initialize_clone(o) # noop end end p [1, 2, 3, 4, 5].clone # => [] ``` ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112539 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). Yeah I was afraid that would be the case, but we could differentiate between user-defined clone callbacks and builtin callbacks. Either way we can probably worry about this later, I was just bringing it up in case you hadn't thought about it already. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112548 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). Yeah, it's really tricky. There has been some talks of just removing the `move: true` capability. Given: https://bugs.ruby-lang.org/issues/21208, I'm not sure it can ever be made correct, hence it might not be worth trying to improve it. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112549 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). That would be fine with me, because actually `move: true` doesn't really "move" the object anymore, it just deep copies it since your change. It's not a less expensive operation anymore, so we may as well keep the original object around if invalidating it could cause issues. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112553 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier).
doesn't really "move" the object anymore, it just deep copies it since your change. It's not a less expensive operation anymor
As mentioned previously, this isn't meant as an end state, but as a quick fix to allow further experimentation. But either way, even with a real "move" we still need to allocate new object slots and traverse the object graph to update the references, so I don't think it ever was significantly cheaper than `move: false`. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112554 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by luke-gru (Luke Gruber). I reread my message and I didn't mean to come across like I disagreed with your changes, because I don't :) ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112555 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). No offense taken. ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112561 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
Issue #20255 has been updated by byroot (Jean Boussier). For the record, I went back to a lower level copying code, but made it size pool aware: https://github.com/ruby/ruby/pull/13070 ---------------------------------------- Bug #20255: Embedded arrays aren't moved correctly across ractors https://bugs.ruby-lang.org/issues/20255#change-112576 * Author: luke-gru (Luke Gruber) * Status: Closed * Assignee: ko1 (Koichi Sasada) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- `ractor.send(ary, move: true)` works incorrectly because if `ary` is embedded, the new moved object doesn't populate its own embedded space, it uses the MovedObject's embedded space. example: ```ruby r = Ractor.new { inner_ary = receive values = {} values[:equal] = (inner_ary == ["",{},2,3,4,5,6]) values[:string] = inner_ary.to_s values } ary = [String.new,Hash.new,2,3,4,5,6] r.send(ary, move: true) r_values = r.take p r_values[:equal] p r_values[:string] # => false # => "[\"\", {}, 2, 2.0, 21747991570, String, 3]" ``` -- https://bugs.ruby-lang.org/
participants (3)
-
byroot (Jean Boussier) -
ko1 (Koichi Sasada)
-
luke-gru (Luke Gruber)