Here's the traceback from the fixtures.rb file failing on test load to the point where it calls Lotus::View.load!, which is responsible for freezing the Hash in question:

/Users/headius/projects/lotus-view/lib/lotus/view/inheritable.rb:41:in `load!'
/Users/headius/projects/lotus-view/lib/lotus/view/dsl.rb:334:in `load!'
/Users/headius/projects/lotus-view/lib/lotus/view/rendering.rb:248:in `load!'
/Users/headius/projects/lotus-view/lib/lotus/view/configuration.rb:363:in `block in load!'
/Users/headius/projects/jruby/lib/ruby/stdlib/set.rb:283:in `each_key'
/Users/headius/projects/jruby/lib/ruby/stdlib/set.rb:283:in `each'
/Users/headius/projects/lotus-view/lib/lotus/view/configuration.rb:363:in `load!'
/Users/headius/projects/lotus-view/lib/lotus/view.rb:290:in `load!'
/Users/headius/projects/lotus-view/test/fixtures.rb:356:in `<top>'

This line corresponds to the Store module in the fixtures, which is the first fixture module to duplicate and load! the Lotus::View module. I suspect we're not duplicating the singleton class for the module and should be.

Ok, I've finally reduced it! It appears we're always putting class variables assigned in module methods on the lexically-enclosing module, rather than on the actual self module they are called against.

module A
  def cvar
    @@cvar = 1
  end
end

class B
  include A.dup
end

B.new.cvar

p A.class_variables # [:@@cvar] on JRuby, [] on MRI

This behaves incorrectly as far back as 1.6.8, so this doesn't appear to be a new bug by any means.

👍

Well this turns out to be a really big issue.

It appears that in MRI, when you dup a module or class, it also duplicates all methods and physically rewrites the new copies to behave as though they were lexically defined within the dupe. That allows it to scope these class vars properly, since class vars scope similar to constants.

As @enebo puts it, we'd consider it "highly undesirable" to have to clone entire method bodies to emulate this behavior, but I'm having trouble thinking of an alternative solution. Note that this cloning likely also affects other lexically-scoped behaviors like refinements.

Maybe @subbuss or @chrisseaton have ideas on how we can solve this.

I would ask this question: since methods are ruby objects and can have state, is there any piece of method object state that cannot be adequately represented across the dup-ed uses without duplicating them?

@subbuss A key example is call sites for refined methods, which will need the proper (new) lexical enclosure. The class variable stuff could probably get away with just having a different StaticScope to use with the same IR.

There is one small piece of good news out of this, I suppose: cref appears to be appropriate for locating where the class vars should go. We're just using the wrong cref, since the dup'ed module's methods think they're still in the original.

Hmm, we may not need to clone call sites after all. Here's an example that I'd expect to use the new refinement Refine2 under MRI's instruction-cloning, but it still uses the original lexical refinement Refine1:

module Refined
  refine(String) { def bar; puts :bar; end }
end

module Refine2
  refine(String) { def foo; puts :quux; end }
end

class A
  using Refined
  def foo; 'str'.bar; end
end

A.new.foo # => bar

B = A.dup
class B
  using Refine2
end

B.new.foo # => also bar, but I expected quux

Of course it's possible they do clone the refined site but they carry the original lexical container along...

Ok...so the key logic happens in rb_vm_rewrite_cref_stack. This function is called after the method (and its instruction sequence) have been cloned, and it does the following:

While recursing up the cref stack...

• Iff the klass for the cref is set to the old mod/cls...
  • Create a new cref.
  • Set the new cref's klass for to the new module.
  • Copy the "overlay module" (refinements) from the original cref and mark both crefs as having a shared OMOD (flags field in rb_cref_t).
  • Return the new cref.
• Otherwise...
  • Create a new cref.
  • Set the new cref's klass to the same as the old cref.
  • Copy the original cref's overlay module into the new cref.
  • Set the current cref to the new cref and proceed to the next element.

I believe this can be summarized like this:

When cloning a module, all instructions are duplicated, and the cref for the entire sequence must be copied:

1. Start with the instruction sequence's immediate cref.
2. If that cref is associated with the module we're cloning, create a new cref with the cloned module as klass, the original overlay module, and shared overlay flags set. Return this as the new cref for the new instruction sequence.
3. If the cref is not associated with the module we're cloning, duplicate its cref (same klass and overlay module) for the new sequence and mark it as having a shared overlay module if an overlay module was present in the original. Set the current cref to this new cref and proceed up the cref stack until 2 applies.

This is hard to explain, but I guess it's basically fixing up all crefs up to the module we're cloning so that they eventually terminate on the new module. If they're already scoped to a different module, they're mostly left alone. Once we find our source module's cref in the stack, we modify that one to point at the new module and we're done.

I believe that MRI does get call site cloning as part of this process, since I think their inline cache is just a slot on the call instruction. This needs to be confirmed, however, as it could affect performance of call sites in dup'ed modules.

Phew .. I need to digest all this. At first glance, it seems like all the cloning is just a side effect of MRI maintaining state in the generated code. Not sure if that applies to how JRuby runtime / callsite caching works. But, that is just an off-the-cuff remark without a whole lot of thought behind it.

But, however, a related thought is that this need not be a blocker for 9k RC1 since this is not a new issue?

@subbuss @enebo I believe yes. This has apparently never worked right in JRuby. It will mean that our RC can't run Lotus (without modifications to Lotus) but it would never have worked on JRuby, and it will never work on Rubinius either as it stands.

When we do fix it, I still believe we can get away with having the same instructions using cloned and modified StaticScope.

When we do fix it, I still believe we can get away with having the same instructions using cloned and modified StaticScope.

I think so too .. as long as you don't maintain scope-specific state in the call-site caches.

I think refinements MUST rewrite some state and it is lexical in nature so it makes sense to fiddle with static/dynamicscope, but I am less sure cvars are lexical. While doing this may be necessary to appropriately support refinements we may be able to make an algorithm which works for only crefs. Does anyone have a lexical example of cvars?

I used findInstanceMethodContainer and it largely only failed in cases where there was a singleton scenario. We know from our current cvar code we need to cope with singletons so perhaps we just need to make a findXXXer which copes properly with singletons?

With having said all that the idea of rewriting instructions does have the potential to eliminate a lot of logic at runtime.

haha...I just realized findInstanceMethod is in terms of lexical constructs, but I am still not sure you cannot derive cvars from self + inheritance chain...

ok I convinced myself that for cvardecl at least it must be lexical:

module Foo
  def a
    puts "a self = #{self}"
    @@a = 1
  end

  def b
    puts "b self = #{self}"
    class << self
      puts "b singleton self = #{self}"
      @@b = 2
    end
  end
end

module Bar
  def a
    puts "a self = #{self}"
    @@a = 1
  end

  def b
    puts "b self = #{self}"
    class << self
      puts "b singleton self = #{self}"
      @@b = 2
    end
  end
end

class A
  include Foo
end

C = Bar.dup

class B
  include C
end


a = A.new
a.a
a.b
puts "A: #{A.class_variables(false)}"
puts "Foo: #{ Foo.class_variables(false)}" # on A
b = B.new
b.a
b.b
puts "B: #{B.class_variables(false)}"
puts "C: #{C.class_variables(false)}"
puts "Bar: #{Bar.class_variables(false)}"

with output:

a self = #<A:0x6fd02e5>
b self = #<A:0x6fd02e5>
b singleton self = #<Class:#<A:0x6fd02e5>>
A: []
Foo: [:@@a, :@@b]
a self = #<B:0x5f2108b5>
b self = #<B:0x5f2108b5>
b singleton self = #<Class:#<B:0x5f2108b5>>
B: []
C: []
Bar: [:@@a, :@@b]

So self is in terms of what you would think self would be but cvars need to go to their lexical equivalent BUT the dup completely hoses it all up.

Any news around this "issue"?

We are aware of what is wrong and MRI made this behavior so long ago that we can no longer complain about this behavior but fixing it is far from trivial and we have been pondering the best way to do that.

@enebo Thanks for the explanation!