In RubyModule's addClassProvider:
This method may contain an instance of double-checked locking. This idiom is not correct...

I'm not worried right now about this particular usage, since, as things currently stand, a) there will be at most one ClassProvider for a module, and b) re-initialization would be harmless, as there's only one instance of ClassProvider, period. (Some may recall this was part of my little hack to enable the double-colon syntax for opening Java classes - I sort of over-engineered this bit )

However, there is a more worrisome instance of this usage in RubyObject#getInstanceVariables. Unless someone has a better idea, I'm going to just make that a synchronized method.

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Basically, double-checked locking is completely broken, plain and simple, and instances where it exists can cause essentially arbitrary broken behavior (use of created-but-uninitialized (constructor not run, or run incompletely) objects, among other things). It's worth fixing. I'll try to upload a patch if I get a chance shortly – alas have been quite busy recently...

I reviewed the patch – it's straightforward and looks fine, but someone more familiar with the compiler/lexer/etc. sections of the codebase should probably double-check (no pun intended) before applying.

I've held off on the more critical (I believe) patch to remove the double-check from RubyObject#getInstanceVariables, in the hope that someone can suggest an alternative to synchronizing the method. Doing so, i.e.

public synchronized Map getInstanceVariables() {
        if (instanceVariables == null) {
            instanceVariables = Collections.synchronizedMap(new HashMap());
        }
        return instanceVariables;
    }

results in about a 5% degradation in instance variable access speed, as measured by the attached benchmark (bench_var.rb). I looked at the possibility of two methods, with an unsynchronized version that would call the synchronized one; but as I read the DoubleCheckedLocking document, we'd end up with the same problem. True?

I should note that overall run times of ant test-interpreted and test-compiled were essentially unaffected by the change (they actually ran a tiny bit faster with the change in a couple of trials). Not that that's a real benchmark, but it might help put the impact of this change in context.