Feature #8570: Better mechanisms to safely load classes concurrently

• Status: Open
• Assignee: -
• Target version: -

[headius (Charles Nutter)]

Today I had an issue reported under JRuby where a user was doing require "some library" unless defined?(SomeClassLibraryDefines). They were running into cases where threads hitting this logic concurrently might see a partially-initialized.

This pattern is not uncommon, and it is broken under all Ruby implementations. I believe this is a major flaw in the way Ruby makes classes visible, and we need to think about changes to how constants are defined during class init or come up with better options for concurrent loading. This bug offers a few ideas and experiments I've tried in hopes we can find something that will work.

The most basic problem is that the constant pointing at the class is set immediately upon opening the class. So the first option is:

1. Do not define the constant until leaving the class/module body.

This option would prevent concurrent threads from seeing an uninitialized class. In essence, this code:

class Foo
def method1 ...
def method2 ...
end

would operated identically to this code:

Foo = Class.new do
def method1 ...
def method2 ...
end

A defined? check on Foo would return false until the class was completely initialized, and lazily require + defined would work correctly.

However, there are pieces of code out there that depend on being able to access the constant from within the class/module body. This brings me to a second option I experimented with on JRuby:

2. Make the constant visible only to the defining thread until leaving the class/module body.

In JRuby, I accomplished this by using Clojure's STM to implement the constant table. The beginning of opening a new class would start an STM transaction. The new, empty class body would be written into the constant table as part of the transaction, only visible to the current thread. Once the class body closed, the transaction would be committed and all code would see a complete class at that constant.

This version solves most issues with #1. The constant is available to that thread, so if code loading or class definition depends on the constant being available, it will still work. It prevents other threads from seeing a partially-initialized class. And it does not interrupt the normal flow of the program.

There are down sides, though, that may or may not happen in practice. If two threads try to start defining a class and it has never been defined before, only one will win. If code loading or class definition logic within the class/module spin up other threads, they won't be able to see the constant.

So then, perhaps we simply need a better mechanism to know if a given class is "complete" or not?

3. Add an attribute to Module indicating whether the class is "open" somewhere in the system.

This would change the require check above to look more like this:

require 'some library' if !defined?(SomeLibrary) || SomeLibrary.open?

The logic here says "if SomeLibrary is not defined or is still being defined, attempt to do the require, which should block until done loading".

But this is somewhat ugly, so I have a fourth suggestion:

4. Add a new mechanism for concurrent requires that understands the services those requires provide.

This would be something like:

require_service(:some_service_name, "some library")

This would behind the scenes perform checks for whether some_service_name was completely loaded as a service, and only define mark that service as "loaded" once the require had completed. This fixes all issues with all scenarios above. However, this basically ends up being:

require "some library" unless $".include?("some library")

...but with a user-provided name rather than the filename going into the equivalent of LOADED_FEATURES.

So...these are some brainstorming ideas. What do you think, Ruby world?

[normalperson (Eric Wong)]

"headius (Charles Nutter)" [e-mail] wrote:

Issue #8570 has been reported by headius (Charles Nutter).

---

Feature #8570: Better mechanisms to safely load classes concurrently
https://bugs.ruby-lang.org/issues/8570

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category:
Target version:

Today I had an issue reported under JRuby where a user was doing require "some library" unless defined?(SomeClassLibraryDefines). They were running into cases where threads hitting this logic concurrently might see a partially-initialized.

This pattern is not uncommon, and it is broken under all Ruby implementations. I believe this is a major flaw in the way Ruby makes classes visible, and we need to think about changes to how constants are defined during class init or come up with better options for concurrent loading. This bug offers a few ideas and experiments I've tried in hopes we can find something that will work.

The most basic problem is that the constant pointing at the class is set immediately upon opening the class. So the first option is:

1. Do not define the constant until leaving the class/module body.

2. Make the constant visible only to the defining thread until leaving the class/module body.

I think 1) and 2) are on the right path.

There are down sides, though, that may or may not happen in practice. If two
threads try to start defining a class and it has never been defined before,
only one will win. If code loading or class definition logic within the
class/module spin up other threads, they won't be able to see the constant.

So then, perhaps we simply need a better mechanism to know if a given class is
"complete" or not?

So the insertion of a new class will need a namespace lock (just like
creating a new file in a directory). This namespace lock is only held
long enough to insert a blank class/module into the namespace, so
it shouldn't be contended.

But once the new class (before methods/constants are defined) exists,
the winner takes a mutex for that class and the loser sleeps on that
mutex

3. Add an attribute to Module indicating whether the class is "open"
   somewhere in the system.

But this is somewhat ugly, so I have a fourth suggestion:

How about doing something like 3), but done behind-the-scenes with
an internal mutex for each class?

thread 1 | thread 2
--------------------------------------------+----------------------------
require "foo" | require "foo"
Object.ns_lock # win | Object.ns_lock # wait
Object.const_get(:Foo) => nil | ...
foo = Class.new | ...
Object.const_set(:Foo, foo) | ...

important: we lock foo before unlocking |

Object.ns_unlock if we created foo |

foo.lock | ...
Object.ns_unlock | ...

foo is still locked | ... # Object.ns_lock completes

                                         | foo = Object.const_get(:Foo)
                                         | # here, foo exists, so we
                                         | # release the Object lock
                                         | # before sleeping on foo.lock
                                         | Object.ns_unlock
                                         | foo.lock # wait

class Foo | ...
def m1 | ...
... | ...
end | ...
end | ...
foo.unlock | ...
| ... # foo.lock completes
| class Foo
| def m2
| ...
| end
| end
| foo.unlock

If m1 and m2 are identical in name (but not implementation),
it'll be down to mutex ordering (but I don't think it's a real issue).
The per-class mutex might need to be recursive to be compatible with
existing code

4. Add a new mechanism for concurrent requires that understands the
   services those requires provide.

Ugh, no. I think it's too complicated and ugly.

Do we agree require is not "hot" enough to need multi-core concurrency
on the same class? I think serializing it transparently is easiest.

[headius (Charles Nutter)]

normalperson (Eric Wong) wrote:

So the insertion of a new class will need a namespace lock (just like
creating a new file in a directory). This namespace lock is only held
long enough to insert a blank class/module into the namespace, so
it shouldn't be contended.

But once the new class (before methods/constants are defined) exists,
the winner takes a mutex for that class and the loser sleeps on that
mutex

The sequence of events here sounds sorta like how autoload was made "thread safe".

Possible states for a class/module in the constant table:

1. Undefined. Opening the class from any thread will not block and transitions to 2.
2. Opened for the first time. Opening the class from any other thread will block until transition to 3.
3. Defined and not open. Opening the class from any thread will transition to 4.
4. Opened for subsequent times. Opening the class from any other thread will block until transition to 3.

In essence, it would mean the following two changes:

• Only one thread could ever be modifying a class.
• Until after the class has been opened and closed for the first time, it is not visible in constant table as a usable class except to the thread that has it open. This prevents instantiating and using it before it has completed initial definition. It would be visible as a "larval" class during this time, so other threads attempting to open it would have to wait.

There's some potential for deadlock if two threads attempt to define the same two classes at the same time in different orders, but they'd have to be doing something like this:

Thread 1:

class Foo
class ::Bar
...
end
end

Thread 2:

class Bar
class ::Foo
...
end
end

That seems unlikely. If there's concerns about this, the lock acquired during class opening could be a global mutex, as you say. The Thread.exclusive lock could be used for this purpose, perhaps? (In JRuby, it is just a global mutex, rather than immediately preventing other threads from running).

Do we agree require is not "hot" enough to need multi-core concurrency
on the same class? I think serializing it transparently is easiest.

I agree. I can think of zero use cases for being able to open and modify a class in two threads at once, and preventing "larval" classes from being usable would help limit concurrent loading issues.

[hsbt (Hiroshi SHIBATA)]

• Project changed from 14 to Ruby master