ruby-core

"headius (Charles Nutter)" <headius@headius.com> 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.

<snip>

> 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.

Thread

Prev Next

In This Thread

Prev Next