Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-volatile instance variables #5187

merged 2 commits into from
Jun 12, 2018


Copy link

@headius headius commented May 23, 2018

So it turns out that since we turned on reified instance variables by default, those variables have not been volatile. This is actually good news, since we wanted to move away from a full volatility guarantee on instance variables in favor of explicit declaration (attr_accessor :foo, volatile: true) or libraries like concurrent-ruby.

This PR simply flips the switch for the remaining varTable-based instance variables, bringing them in line with field-based instance variables as far as volatility goes.

I have also filed #5186 to finish the reification project and add back the ability to turn on volatility as desired. This will be needed in any case to support declarative volatility in a future update to Ruby.

The field-based variables we used by default during 9.1.x already
were not volatile, which is the direction we want to move people.
Volatility should be achieved through libraries like
concurrent-ruby, or added to Ruby as a way to declare volatility
at class definition time (attr_accessor :foo, volatile: true).

The risk here seems small since 99% of variable accesses have
already been nonvolatile for over a year, all major concurrency
libraries are using the proper mechanisms that don't require
instance variable volatility, and we have received no bugs where
nonvolatile field-based variables were the cause.
@headius headius added this to the JRuby milestone May 23, 2018
@enebo enebo modified the milestones: JRuby, JRuby May 24, 2018
@headius headius merged commit d4b980f into jruby:master Jun 12, 2018
@headius headius deleted the nonvolatile_ivars branch June 12, 2018 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants