Rails5, auto-loading, future, circular dependency

[jrochkind]

My first time trying to use concurrent-ruby in Rails5 and with MRI 2.3. MRI 2.3.1. concurrent-ruby 1.0.2. Running into something very odd.

future = Concurrent::Future.execute { something_that_will_cause_rails_autoloading }
future.value

Hangs forever, never comes back. It's only by some weird debugging that I discovered it's "something_that_will_cause_autoloading" -- and I'm not certain of this, but I suspect it was Rails auto-loading behavior, How? Before calling future.value, I did a weird future.instance_variable_get('@executor').shutdown. After I do that, if I call future.value, it does return, but has failed -- and future.reason is #<RuntimeError: Circular dependency detected while autoloading constant BentoSearch::ResultItem> (which is one of my own classes)

So some thoughts:

1. I am not sure why it's deciding there's a 'circular dependency', I can require 'bento_search/result_item' on it's own (not in a future) fine, with no warnings.
2. I suspect I am doing something Rails doesn't like, perhaps you need to use some kind of eager loading if you are going to use concurrency in Rails 5?
3. But even if I'm doing something wrong, why is the future hanging forever? What's actually going on? I'm not sure. I'd expect it to come back with an exception -- perhaps that "circular dependency" exception, why did I end up with that if I reached in to find the executor and shut it down myself, but a hang forever otherwise? It seems like concurrent-ruby is maybe doing something wrong.... maybe?

Wow, concurrency is confusing. Any thoughts.

[pitr-ch]

@jrochkind Thanks for letting us know! We definitely pay attention to rails + c-r combination. Could you create a reproducer for this? The blocking part is enough.

[jrochkind]

Upon further investigation, I think it is about Rails auto-loading, and is all about Rails, and not at a ruby-concurrent bug or something ruby-concurrent can do anything about.

But is something users of ruby-concurrent in a Rails app will have to know about, that you may want to give them hints on or be able to answer questions on.

Repro with giant write-up here, also reported as Rails issue.

Basically, using concurrency in the request-loop in Rails5 seems to require turning off dev-mode class reloading, and the 'failure mode' if you fail to do so looks like a deadlock.

[jrochkind]

Okay, I've figured some stuff out by reading source, expirimentation, but mostly by @matthewd answering my questions.

Rails 5 has several new architectural components and API for dealing with concurrency.

The good news is that can make auto-loading, and even dev-mode reloading, concurrency-safe for the first time ever.

The bad news it's pretty much entirely undocumented (not even method-level rdoc mostly), and if you don't use it correctly the 'failure mode' can be a deadlock in your application. As opposed to Rails4, where autoload concurrency failure meant weird exceptions, now it can often mean deadlock.

The 'correct' way to do some concurrency inside a Rails action method and then wait for the work to complete before returning a response:

  # a controller action method
  def show
    futures = 3.times.collect do |i|
      Concurrent::Future.execute do
        Rails.application.executor.wrap do
          SomeWorker.new.value
       end
      end
    end

    ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
      @example_values = futures.collect(&:value)
    end
  end

• ActiveSupport::Dependencies.interlock.permit_concurrent_loads

  In cases where autoloading happens (an actual autoload is triggered, an autoloadable constant not yet loaded is accessed), if you don't do this, you wind up with deadlock, where the threads are trying to autoload, but can't, because the Rails request/action/worker has some type of lock on autoloading. The future threads are waiting for the lock to be relinquished, but the worker thread with the lock is waiting for the threads to be complete.

  So in those cases, the threads will actually be blocked until we do permit_concurrent_loads, at which point they'll be allowed to do their thing, and we wait for them to complete inside that block.

  If autoloading isn't happening, this isn't necessary. So for instance, if Rails config.eager_load == config.cache_classes == true (as normally in production, but not dev), then I don't think it's neccesary.

  It probably doesn't hurt even when used where not necessary.

• Rails.application.executor.wrap

  Rails new API for signaling intent 'to do some work'. One thing it does is make autoloading actually work concurrency-safe. If you don't have this line here, and autoloading happens inside the future (actually is triggered, not just an auto-loadable constant, but one which has not yet been loaded), then one or more futures may raise RuntimeError: Circular dependency detected while autoloading constant NameOfClass. I am not sure why this exhibits with a message "Circular dependency" or what's "circular" about it, but it means "you didn't autoload in a concurrency safe way".

  Rails.application.executor.wrap around your work is the way to make autoloading concurrency safe. But it's intended to be used regardless of dev or production mode, it's always intended to be used around 'Rails-related work being done' in a thread.

  It may eliminate the need for ye old ActiveRecord::Base.connection_pool.with_connection, the wrap takes care of checking connections back in at the end of the block for you. Needs more verification.

  There's another version, Rails.application.reloader.wrap. I believe the functionality there is a superset of Rails.application.executor.wrap, but I'm not totally sure in what ways. But you can't use it in this case, since a Rails request loop is already wrapped in a Rails.application.reloader.wrap, using one here... seems to result in deadlock. Sidekiq does use it for it's workers, which I think result in the workers actually getting dev-mode reloading working for them. So that's one extra thing it does.

  If you are in 'production-mode' configuration (usually no auto-loading) what are the consequences of not using Rails.application.executor.wrap? Not sure.

Phew. I haven't fully wrapped my head around how this impacts using gems which use concurrency (with concurrent-ruby or otherwise) that aren't neccessarily written specifically for Rails, so don't have any Rails specific API in them and don't want any -- if they end up triggering any Rails auto-loading, it could be bad and deadlock. They probably wouldn't ordinarily, unless you pass class names into them or something.

Removing certain directories from Rails config.autoload_paths might be one solution, I haven't fully investigated it. It's unclear to me if config.autoload does anything in Rails5.

@matthewd promises some actual docs, which I eager await. If anyone else wants to play around with concurrent-ruby in a Rails app, your feedback or problems you run into might help @matthewd understand his audience and what the docs need to hit.

[jrochkind]

So, yeah, it's not a concurrent-ruby issue... but I think many many people trying to use concurrent-ruby in Rails5 are going to run into these issues, so concurrent-ruby team probably wants to be aware of them, and maybe even doc them especially if Rails doesn't doc them soon.

[ianks]

I stumbled across this issue as well. Spend an afternoon trying to figure out why I started deadlocking. I think dealing with this should be high-priority since it really highers the burden of entry on using concurrent patterns in Rails.

To put this in perspective a little bit, I (as a contributor to this library who actively enjoys studying concurrency), spent hours tracking this bug down. Imagine the pain others may feel.

Let's start brainstorming some solutions:

1. Disable multithreading if Rails.env.development? (not a good solution at all IMO)
2. If a user deadlocks, or some circular dep is detected, give them a specific warning and references about how to fix the issue (better)
3. Always permit_concurrent_loads somehow when blocking CR methods are running (meh... this would still cause auto-load race conditions to occur, but is arguably better than getting stuck on deadlocks).

All in all, this is somewhat leading me to believe that we should create a rails integration gem concurrent-ruby-rails which attempt to address many of these Rails specific concerns.