Introduce ractorize

[andrewn617]

This PR contains the rest of the work to make a fresh Rails application ractor shareable.

We introduce the ractorize! method which prepares the application to be frozen then calls Ractor.make_shareable(self). After this the application can be shared in non-main Ractors to do interesting things like serve requests.

There are a few constraints:

• the application must be eager loaded, no autoloading and no reloading
• ditto for routes
• no action cable
• no solid cache store
• no rack file server (this is fixed in rack but we need a new version cut - and I have a few more rack fixes to push first)

This PR contains a few fixes that make ractorization work:

• The app's key generator is now ractor local (if the app has opted in to ractors). This contains a concurrent map which is not Ractor-shareable. Ractor-safe hashes are still in development, so this is a pragmatic choice for the time being.
• Engines' app must be initialized at freezing time so we can drop the mutex used to control building it
• A number of hash default procs which mutate the hash are inherently not shareable. Those hashes are fully built at sharing time, so we can then drop the default proc.

I put those fixes on individual commits so they're easier to review. We could upstream them separately, though it might be clearer to include them all in one PR.

[andrewn617]

Maybe we should also do @memos.freeze here altho its not necessary when calling Ractor.make_shareable, it seems weird to remove the default proc without freezing the hash.

[etiennebarrie]

Yes it's super weird and something Ractor.make_shareable really doesn't help with because it calls freeze, checks that the object itself is frozen then moves on to freeze objects referenced by the object.
On the one hand, it makes sense, because e.g. [Object.new].freeze.first.frozen? # => false, on the other hand when designing classes it feels not helpful:

class Foo
  def initialize = @foo = []
end
Ractor.shareable? Foo.new.freeze # => false
foo = Foo.new.freeze
Ractor.make_shareable foo
Ractor.shareable? foo # => true

There's no tool to let class authors know that @foo wasn't frozen, that would help them write a comprehensive freeze implementation.

I'm not sure what it should look like though, maybe Ractor.make_shareable(obj, freeze: false) that would raise if any referenced object is not already frozen maybe?

[tenderlove]

We should call freeze. I know it's not necessary if you're going to turn around and call make_shareable, but I usually expect calling freeze on a complex object to freeze its references.

@etiennebarrie Could you just try passing the frozen object to a Ractor and see if it raises an exception? 😅

[andrewn617]

Sure 👍 I made this and the others freeze all their internals.

[etiennebarrie]

This breaks the contract of inheritable options.

h=ActiveSupport::InheritableOptions.new(foo: 42)
h[:foo] # => 42
h.default_proc=nil
h[:foo] # => nil

IIRC I had something like:

def freeze
  replace(to_h)
  super
end

But that is a bit naive, trades memory for performance, we may want to go the opposite way and also make @parent frozen (and the default proc ractor shareable, since this one doesn't mutate, that should work).

[andrewn617]

Yeah sorry 🤦 This doesn't follow the pattern of the others my bad.

I think freezing the @parent would work, but we need to pass it as self of the shareable proc I believe. And it could have unshareable values within so I think we will need to expand edouard's work to have a Ractors.make_shareable_attempt to try to freeze it 🤔

I'll think about it

[etiennebarrie]

Actually if the self of the proc is shareable, and it doesn't capture variables, the proc could in theory be shareable too.
But there's a catch22 where the hash can't be made shareable because it has the proc that is not shareable because the hash is not shareable yet.

So unless I'm missing something, I don't think there's a good way to make a Hash with a default proc shareable, even if the default proc could in theory be shareable.

[andrewn617]

Actually I don't think we can do it in the initializer. People can mutate the parent of course, so we can't freeze it then. We could do:

    def freeze
      Ractors.make_shareable(@parent)
      self.default_proc = if @parent.kind_of?(OrderedOptions)
          ActiveSupport::Ractors.shareable_proc(self: @parent) { |h, k| _get(k) }
        else
          ActiveSupport::Ractors.shareable_proc(self: @parent) { |h, k| self[k] }
        end
      super
    end

That seems to work. But to your other comment above, it feels like we need way to hook into make_shareable, since the behaviour really makes no sense as part of freeze. And actually itll break in ruby 3 if we do this since shareable_proc is a no op, we will still have the inherited option as self in the block so I think we will need to add a check.

[etiennebarrie]

Right, for now I think replace(to_h) is still the best solution we have.

[andrewn617]

Sorry missed your earlier message cause we posted around same time. But yeah, making default proc shareable seems too weird, I think replace(to_h) is fine for beta support you are right.

[andrewn617]

This thing has some APIs beyond a normal hash so I don't think its fair game to just to_h it 🤔 .

Instead I think its safer to merge the parent's entries at freezing time then remove the default proc. That way it still quacks like an inheritable options in all ways.

[etiennebarrie]

Oh right, using replace keeps the class so dynamic accessors work but overridden? will need to be handled separately.

inherited = {foo: 42, bar: 21}
options = ActiveSupport::InheritableOptions.new(inherited)
options.bar = 21
options.baz = 7

assert_equal 42, options.foo
refute options.overridden?(:foo)
assert options.overridden?(:bar)
refute options.overridden?(:baz)
refute options.key?(:qux)

We'll need to keep a copy of the keys before freezing.

[andrewn617]

Yeah I was thinking about this last night........... I think I will keep track of the keys when we call []= that will simplify the overridden

[andrewn617]

Maybe I'll pull this into its own PR since it's becoming a bit involved

[gmcgibbon]

Why a lambda and not a proc?

[andrewn617]

No reason

[tenderlove]

Is key_generator is set by the user config? Also, is this lambda is allocated on every request?

Not a show stopper, but it feels like we could wrap the key generator with a shareable proc when it's set, and cache it. That way we don't have to allocate a new proc on every request.

[jhawthorn]

It also looks like just the "caching" part of CachingKeyGenerator that may be the problem with shareability? Maybe we could swap it for the non-caching version, or make the cache have a Ractor-local or Thread-local store?

[andrewn617]

Also, is this lambda is allocated on every request?

I don't think so. This is in the @app_env_config on the application which we freeze. It gets merged into the request env when we build the request but I don't think should allocate anything new https://github.com/rails/rails/blob/main/railties/lib/rails/engine.rb#L772

It also looks like just the "caching" part of CachingKeyGenerator that may be the problem with shareability?

Correct. I looked at just "freezing" that first by making the internal cache ractor local. But the issue is key_generators is a hash. The default is scoped to secret_key_base, but developer could have others. The issue was I wasn't sure it was a safe assumption these would all be created eagerly at boot time, so if we freeze that hash then we would break anything trying to create one when a request is in flight. That's why its the hash that is ractor local not the cache itself. But maybe it is a safe assumption and we expect people to create them in an initializer 🤔 It does make this all a lot more complicated.

Is key_generator is set by the user config?

No created by us with the default keyed to secret_key_base but there could be others (seems most common use case is old keys being rotated)

[andrewn617]

Thinking about it today, I think could just say no new key generators at request time in Ractor mode - at least to start. I believe we are going to run into the same dilemma with AS notification subscribers 🤔

[tenderlove]

Pulling key_generator to a method in this test seems like a good refactor regardless of Ractor safety stuff. Should we pull this change in to a different PR so we can reduce the diff here?

[andrewn617]

rails#57663

[tenderlove]

We should call freeze. I know it's not necessary if you're going to turn around and call make_shareable, but I usually expect calling freeze on a complex object to freeze its references.

@etiennebarrie Could you just try passing the frozen object to a Ractor and see if it raises an exception? 😅