Issue #22291 has been updated by Eregon (Benoit Daloze). jhawthorn (John Hawthorn) wrote:

I propose that we fix this by forbidding instance variable writes to objects which are frozen.

... to objects which are shareable, right? (it's already forbidden on frozen objects) I think this makes sense but it makes me wonder, should making these objects shareable also freeze them? (e.g. for Ractor, Ractor::Port, Ractor.shareable_proc, ENV) That would simplify the model. Classes/Modules are already special so that would be the exception we probably need to keep. IIRC @ko1 once said that Ractor instances being shareable yet not frozen was intentional to allow ivars on them. ---------------------------------------- Bug #22291: Instance variables should be forbidden on Ractor-shareable objects https://bugs.ruby-lang.org/issues/22291#change-118782 * Author: jhawthorn (John Hawthorn) * Status: Open * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- Usually Ractor-shareable objects are frozen, but we have a number of objects which are shareable even though they aren't frozen (and likely will have more in the future from C extensions). * Classes/Modules (ignored in this issue, they have special ivar handling) * `Ractor` * `Ractor::Port` * Isolated procs/lambdas from `Ractor.shareable_proc` * `ENV` These are set shareable without the usual checks that all referenced objects are also shareable. This causes a potential issue, because these objects could have unshareable instance variables, breaking the Ractor invariant (potentially causing race conditions and segmentation faults). Currently, we attempt to avoid this by having a check on ivars read that forbids reading these from a non-main Ractor ``` R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") Ractor.new { R.instance_variable_get(:@iv) }.value #=> Ractor::IsolationError: can not access instance variables of shareable objects from non-main Ractors ``` This is detected by checking on ivar read for objects which are shareable but **not** frozen. However this isn't sound because most of these unshareable objects can be frozen, turning the check off. ```ruby R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") R.freeze Ractor.new { R.instance_variable_get(:@iv) }.value # => no error, created Ractor got access to an object it shouldn't ``` (weird quirk: `ENV#freeze` raises, but you can `Kernel.instance_method(:freeze).bind_call(ENV)` so it's still an issue) I propose that we fix this by forbidding instance variable writes to objects which are frozen. This removes the need to check on read and maintains the Ractor invariant. ``` R = Ractor.new {} R.instance_variable_set(:@iv, 123) #=> Ractor::IsolationError: can not set instance variables of shareable Ractor objects Ractor.new { R.instance_variable_get(:@foo) # => always nil, because ivars are forbidden } ``` These objects behave essentially as though they're frozen, but only the IVs are frozen. This is an important issue to solve both for correctness, and because I want us to decide the semantics so that they can be implemented in ZJIT. Currently ZJIT won't compile ivar reads on multi-ractor mode because of this issue. (I will link a patch implementing this shortly) -- https://bugs.ruby-lang.org/

Issue #22291 has been updated by jhawthorn (John Hawthorn). Description updated Thanks for the correction, edited the description. Eregon (Benoit Daloze) wrote in #note-1:

IIRC [@ko1 (Koichi Sasada)](/users/17) once said that Ractor instances being shareable yet not frozen was intentional to allow ivars on them.

I discussed this with @ko1 and this didn't come up. I don't think it ever makes sense currently to set instance variables on a Ractor (which currently can only be done by the main Ractor). I'm not proposing any changes to Classes/Modules (#22226 is an accepted proposal to make a change there, this issue should not affect or be affected by that) --- Both comments above suggest freezing some or all of these objects. That would solve the invariant violation, but I think there's a question of what "freezing" is supposed to represent. Whether it's just that instance variables are immutable, or if the object's specific state is immutable as well. Looking at some specifics: * Freezing Ractor.shareable_proc seems fine to me * Ractor is a lot like Thread. You can freeze a Thread, which freezes thread and fiber local variables. (execution still runs, it's a bit of a weird thing to do) * Ractor::Port is a lot like Queue. Queue as of #17146 (from @Eregon) can't be frozen, which makes Ractor::Port being frozen by default seems strange. Seems like it was a workaround in [b9188901c07649c3af3a5f925ec0dead444a4134](https://github.com/ruby/ruby/commit/b9188901c07649c3af3a5f925ec0dead444a4134), but we'd have to ask @ko1 * ENV forbids `.freeze`, introduced in #15920, however it can be frozen via bind_call (see #17738). This is acceptable for avoiding user confusion, but would not be sufficient for Ractor's safety guarantees. When we make this decision we also need to consider other Ractor-safe objects and data structures that a C extension might define. We haven't yet published a proper API for this, but it would be weird for ex. a "Ractor safe hash" to be frozen. IMO our past decisions suggest that frozen means that a Ractor-safe object's state should also be frozen, and not just the ivars, and so we should just forbid ivars on those objects. ---------------------------------------- Bug #22291: Instance variables should be forbidden on Ractor-shareable objects https://bugs.ruby-lang.org/issues/22291#change-118811 * Author: jhawthorn (John Hawthorn) * Status: Open * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- Usually Ractor-shareable objects are frozen, but we have a number of objects which are shareable even though they aren't frozen (and likely will have more in the future from C extensions). * Classes/Modules (ignored in this issue, they have special ivar handling) * `Ractor` * `Ractor::Port` * Isolated procs/lambdas from `Ractor.shareable_proc` * `ENV` These are set shareable without the usual checks that all referenced objects are also shareable. This causes a potential issue, because these objects could have unshareable instance variables, breaking the Ractor invariant (potentially causing race conditions and segmentation faults). Currently, we attempt to avoid this by having a check on ivars read that forbids reading these from a non-main Ractor ``` R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") Ractor.new { R.instance_variable_get(:@iv) }.value #=> Ractor::IsolationError: can not access instance variables of shareable objects from non-main Ractors ``` This is detected by checking on ivar read for objects which are shareable but **not** frozen. However this isn't sound because most of these unshareable objects can be frozen, turning the check off. ```ruby R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") R.freeze Ractor.new { R.instance_variable_get(:@iv) }.value # => no error, created Ractor got access to an object it shouldn't ``` (weird quirk: `ENV#freeze` raises, but you can `Kernel.instance_method(:freeze).bind_call(ENV)` so it's still an issue) I propose that we fix this by forbidding instance variable writes to objects which are shareable (whether or not thy are frozen). This removes the need to check on read and maintains the Ractor invariant. ``` R = Ractor.new {} R.instance_variable_set(:@iv, 123) #=> Ractor::IsolationError: can not set instance variables of shareable Ractor objects Ractor.new { R.instance_variable_get(:@foo) # => always nil, because ivars are forbidden } ``` These objects behave essentially as though they're frozen, but only the IVs are frozen. This is an important issue to solve both for correctness, and because I want us to decide the semantics so that they can be implemented in ZJIT. Currently ZJIT won't compile ivar reads on multi-ractor mode because of this issue. (I will link a patch implementing this shortly) -- https://bugs.ruby-lang.org/

Issue #22291 has been updated by jhawthorn (John Hawthorn). matz (Yukihiro Matsumoto) wrote in #note-4:

Since `ENV` is not an experimental API, please check the impact on existing code that sets instance variables on it.

I investigated this pretty thoroughly and found only one active gem, environment_helpers. I submitted a patch and that's already been released in a new version https://github.com/nevinera/environment_helpers/pull/50 Other gems I found that I don't think are blockers: usepropeller/motion-env (targets RubyMotion), ancat/trapdoor (non-functional), ancat/smuggle-env (renamed to trapdoor), csquared/smart_env (no release in 15 years) ---------------------------------------- Bug #22291: Instance variables should be forbidden on Ractor-shareable objects https://bugs.ruby-lang.org/issues/22291#change-119042 * Author: jhawthorn (John Hawthorn) * Status: Closed * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- Usually Ractor-shareable objects are frozen, but we have a number of objects which are shareable even though they aren't frozen (and likely will have more in the future from C extensions). * Classes/Modules (ignored in this issue, they have special ivar handling) * `Ractor` * `Ractor::Port` * Isolated procs/lambdas from `Ractor.shareable_proc` * `ENV` These are set shareable without the usual checks that all referenced objects are also shareable. This causes a potential issue, because these objects could have unshareable instance variables, breaking the Ractor invariant (potentially causing race conditions and segmentation faults). Currently, we attempt to avoid this by having a check on ivars read that forbids reading these from a non-main Ractor ``` R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") Ractor.new { R.instance_variable_get(:@iv) }.value #=> Ractor::IsolationError: can not access instance variables of shareable objects from non-main Ractors ``` This is detected by checking on ivar read for objects which are shareable but **not** frozen. However this isn't sound because most of these unshareable objects can be frozen, turning the check off. ```ruby R = Ractor.new {} R.instance_variable_set(:@iv, +"mutable") R.freeze Ractor.new { R.instance_variable_get(:@iv) }.value # => no error, created Ractor got access to an object it shouldn't ``` (weird quirk: `ENV#freeze` raises, but you can `Kernel.instance_method(:freeze).bind_call(ENV)` so it's still an issue) I propose that we fix this by forbidding instance variable writes to objects which are shareable (whether or not thy are frozen). This removes the need to check on read and maintains the Ractor invariant. ``` R = Ractor.new {} R.instance_variable_set(:@iv, 123) #=> Ractor::IsolationError: can not set instance variables of shareable Ractor objects Ractor.new { R.instance_variable_get(:@foo) # => always nil, because ivars are forbidden } ``` These objects behave essentially as though they're frozen, but only the IVs are frozen. This is an important issue to solve both for correctness, and because I want us to decide the semantics so that they can be implemented in ZJIT. Currently ZJIT won't compile ivar reads on multi-ractor mode because of this issue. (I will link a patch implementing this shortly) -- https://bugs.ruby-lang.org/