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.

Sign up for GitHub

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

Jump to bottom

Same class variables may return different values on different modules. #3357

Open
pitr-ch opened this issue Sep 28, 2015 · 5 comments
Open

Same class variables may return different values on different modules. #3357

pitr-ch opened this issue Sep 28, 2015 · 5 comments
Labels
JRuby 9000

Comments

@pitr-ch
Copy link
Member

pitr-ch commented Sep 28, 2015

It looks like the current implementation on JRuby is not fully correct. The read and write of the value uses ConcurrentHashMap so if the class-var exists it has volatile semantic, however during definition of a new class-var it appears it's not sufficiently protected so it can lead to class-vars to be created twice not sharing the value as they suppose two.

Update: broken example, please see the one bellow

c = Class.new; c1 = Class.new(c); c2 = Class.new(c)
Thread.new { c1.class_eval { @@a = 1 } }
Thread.new { c2.class_eval { @@a = 2 } }
# then following may be true
c1.class_eval { @@a } != c2.class_eval { @@a }

see https://github.com/pitr-ch/jruby/blob/master/core/src/main/java/org/jruby/RubyModule.java#L3313-L3322
@pitr-ch pitr-ch mentioned this issue Sep 28, 2015
Closed
@eregon
Copy link
Member

eregon commented Sep 28, 2015

The example is flawed, when run under MRI you get (because cvar lookup is based on lexical scope and block-style class_eval does not change it):

Thread.new { c1.class_eval { @@a = 1 } }
(pry):2: warning: class variable access from toplevel

How should it work?
putIfAbsent() in the "not found" (last line) case and try again until it succeeds or we found a superclass having it?

@pitr-ch
Copy link
Member Author

pitr-ch commented Sep 29, 2015

Ah sorry, I've written it in a hurry, the example should be:

loop do
  c = Class.new; c1 = Class.new(c)
  [Thread.new { c.class_variable_set :@@a, 1 },
   Thread.new { c1.class_variable_set :@@a, 2 }].each(&:join)
  print '.'
  if c.class_variable_get(:@@a) != c1.class_variable_get(:@@a)
    p c.class_variable_get(:@@a), c1.class_variable_get(:@@a)
    break
  end
end

This one ends on JRuby 1.7.21 with printing different values.

Could you reformulate your question please? I am not sure what you mean.

@eregon
Copy link
Member

eregon commented Sep 29, 2015

Looks good now, I was just thinking out loud about implementation since it not totally trivial as the class hierarchy must be traversed while looking up class variables.

@eregon
Copy link
Member

eregon commented Sep 29, 2015

This also happens in 9k and Truffle.

@pitr-ch
Copy link
Member Author

pitr-ch commented Sep 29, 2015

It could use single global lock for adding a class variable, assuming class variables are added mostly only during load time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
JRuby 9000
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rtyler @eregon @pitr-ch