application controller (via activeadmin) is not reloaded when modified in dev mode

[bughit]

1. new rails 4.0 project (ruby 2.0.0)
2. add active admin
   1. gem 'activeadmin', github: 'gregbell/active_admin', branch: 'rails4'
   2. rails generate active_admin:install
   3. rake db:migrate
3. start rails
4. navigate to /admin/login
5. add before_action{raise} to applicationcontroller, save
6. reload page, it still loads, applicationcontroller has not been reloaded

[carlosantoniodasilva]

Does it happen without activeadmin, or have you noticed this with any other extension? Maybe they are doing something on the reloading code on their side?

[bughit]

It does not happen with regular app/controllers. I haven't checked other gems with controllers that inherit from application controller.

ActiveAdmin controllers are generated on startup/demand. I am not clear on how const unloading/reloading works with inheritance, when the base changes does Rails need to unload all subclasses? I suppose that if it does, it's impossible to unload and reload controllers that are not file based.

[steveklabnik]

If it doesn't happen without ActiveAdmin, then it's not our bug, it's theirs. You should file a bug over on their tracker.

[bughit]

How does rails modified unloading/reloading handle inheritance, i.e. when a base class changes, does it need to unload all subclasses? I'll investigate at some point, but hopefully someone knows.

I think the answer to this determines whether this scenario can be expected to work at all.

[steveklabnik]

@fxn is the master of that, but I believe it's here? https://github.com/rails/rails/blob/master/activesupport/lib/active_support/dependencies.rb

[fxn]

@steveklabnik correct.

@bughit I would need to dig into it, but this smells like a stale class object being cached.

If there was (I am speculating) an ActiveAdmin controller that inherited from ApplicationController and that admin controller is not being wiped when constants are cleaned for whatever reason, its superclass would be a class object that is no longer reachable using the ApplicationController constant, but that is alive nonetheless.

You would normally want to force redefining that admin controller so that the evaluation of the ApplicationController constant for its superclass would trigger constant autoloading again, to pick the new definition with the filter.

[bughit]

@fxn, from what you wrote, it sounds like the answer to my question:

when a base class changes, does it need to unload all subclasses?

is yes?

So rails also unloads controllers from gems? And then reloads them? It's able to do that? And if these controllers are not in files but are dynamically constructed on startup (as I believe ActiveAdmin controllers are) then this can't work at all? Or does activeadmin need to do something to make this work?

[fxn]

Active Support cannot in general unload subclasses blindly, because it is out of your control how they were loaded and it could be the case that they are not going to be redefined at all in the next request. Also, unless you go via ObjectSpace or something costly like that, there's no API in Ruby to directly ask for your descendants.

Autoloaded subclasses are going to be wiped as any other constant. The key point is that in general, you just cannot have autoloaded ancestors in a regular non-autoloaded class.

Can you point to the place where ActiveAdmin defines these controllers?

[bughit]

I am still not clear on the general strategy. Experimentally it appears that if you modify a base controller (applicationcontroller), the subclasses that are part of project (app/controllers) are reloaded on the next request. How's that accomplished? I am assuming that subclasses of the modified controller are found and unloaded, is that not the case?

And if the subclasses are not part of the project, are they just left left alone? Is that what accounts for the behavior I reported? What I am trying to understand is, given the current modified reloading strategy/algorithm, is current behavior expected and unavoidable, or is there something gems with controllers need to do to make reloading possible?

The controller invloved in /admin/login is:
https://github.com/gregbell/active_admin/blob/rails4/lib/active_admin/devise.rb#L57

Other activeadmin controllers:
https://github.com/gregbell/active_admin/blob/rails4/lib/active_admin/namespace.rb#L181
https://github.com/gregbell/active_admin/blob/rails4/lib/active_admin/namespace.rb#L207

There may be others.

[fxn]

The strategy is the following: Active Support keeps track of constants that have been autoloaded. When a change is detected in the files in development mode (depends on cache_classes really), all autoloaded constants are removed from the module they belong to using remove_const.

Since the constants are no longer there, in the next request they are autoloaded again on demand and classes are thus reloaded.

So, controllers are reloaded just because their constants were autoloaded and then wiped, regardless of their relationship with ApplicationController. That is, the controller ProductsController, and the model Product are reloaded by the same principle.

If you'd like to understand a little better how this works you could watch this talk by such an eloquent and breathtakingly handsome speaker!

In the examples you pointed to, I don't really see autoloaded constants, right?

[bughit]

In the examples you pointed to, I don't really see autoloaded constants, right?

to be clear, by autoloaded, do you mean Kernel#autoload or the Rails autoload_paths mechanism?

Class reloading only happens for constants autoloaded by Active Support. Active Admin defines those constants using Kernel#autoload according to lib/active_admin.rb. Those ones are loaded bypassing AS and thus won't be wiped, unless the root ActiveAdmin namespace is (which seems unlikely because the file is probably required by bundler).

I see that if you follow up the hierarchy you end up in controllers from inherited_resources that inherit from ApplicationController. So in principle you'd say this setup does not play well with AS reloading of ApplicationController.

In principle what Active Admin would need to do is to uniformily delegate constant autoloading to AS dependencies for classes that inherit from AS-autoloaded classes.

But that is a conjecture from a quick read of the source code, let me play a little bit with it and confirm (or deny!). After BaRuCo though.

[thedarkone]

How exactly would for example the SessionsController in the first link need to be defined in ActiveAdmin for it to be reloaded after a modification of the ApplicationController?

Its filepath needs to be reachable through ActiveSupport::Dependencies.autoload_paths (like for any other controller class, note that for example Rails::Engine auto-adds the necessary paths for you..) and it needs to be properly named so that ActiveSupport::Dependencies can infer the filename that it needs to load whenever ActiveAdmin::Devise::SessionsController constant is referenced.

In this case it should be a located in active_admin/devise/sessions_controller.rb (that is reachable through ActiveSupport::Dependencies.autoload_paths). This way whenever you refer to ActiveAdmin::Devise::SessionsController ActiveSupport::Dependencies knows that it needs to load active_admin/devise/sessions_controller.rb. If you do this, the Rails will autoload that controller for you and when it needs it (when any of the .rb files are changed) it will then unload the controller (so that it can auto-loaded on the next request again).

Basically you need to behave exactly the same as if the ActiveAdmin::Devise::SessionsController was in your app/controllers directory.

If you want an example, you can have a look at Tolk it has controllers that are autoloaded (and auto-unloaded) by Rails.

[a-chernykh]

Does someone experiencing this issue could give activeadmin/activeadmin#2906 a try? I believe it should be fixed there.

[seanlinsley]

I haven't tried activeadmin/activeadmin#2906 in years, but even if it did "fix" the problem, there's still weirdness happening. The important thing to note with Active Admin is that the controller classes are dynamically defined at runtime, so they don't have a file to be reloaded from.

Doing some tests, it appears that ActionController::Base holds onto classes that subclass it even after their constant has been removed, and garbage collection has been run multiple times (I assume that's necessary because of the new generational garbage collector).

# Starting from a fresh IRB console, running Rails 4.2.5.1

require 'rails/all'


# ActionController holds onto the class

ActionController::Base.descendants.count => 0

class FooBar < ActionController::Base
end

ActionController::Base.descendants.count => 1

Object.send :remove_const, :FooBar

GC.start
GC.start
GC.start

ActionController::Base.descendants.count => 1
ActionController::Base.descendants       => [FooBar]
FooBar                                   -> uninitialized constant


# Even with Rails loaded, Object doesn't hold onto classes like that

Object.descendants.count => 1441

class FooBar < Object
end

Object.descendants.count => 1442

Object.send :remove_const, :FooBar

GC.start
GC.start
GC.start

Object.descendants.count => 1441
FooBar                   -> uninitialized constant

[seanlinsley]

What's odd about this is that the Mass gem isn't reporting any objects referring to the controller, that might prevent it from being garbage collected.

Same with this setup:

Mass.references ActionController::Base.descendants.first

[seanlinsley]

Even scoping the require to just require 'action_controller', the problem still happens. Though it seems requiring that also loads ActiveSupport::Dependencies.

[thedarkone]

@seanlinsley yes, doing what you are doing is not supported :-(.

Also please note that Object.descendants is not a public API, Object.subclasses is though.

Furthermore ActionController::Base.descendants and Object.descendants are different methods.

PS: don't know if it will be of any help (it depends on what the rest of your app does, it might not be written in a way that is compatible with the gem) - with https://github.com/thedarkone/rails-dev-boost you should be able to do this:

class FooBar < ActionController::Base
end

ActionController::Base.descendants.count => 1

Object.send :remove_const, :FooBar

ActionController::Base.descendants.count => 0

[seanlinsley]

Furthermore ActionController::Base.descendants and Object.descendants are different methods.

Ah, thanks for pointing that out.

Object.method(:descendants).source_location
 => ["activesupport-4.2.5.1/lib/active_support/core_ext/class/subclasses.rb", 8] 
ActionController::Base.method(:descendants).source_location
 => ["activesupport-4.2.5.1/lib/active_support/descendants_tracker.rb", 56] 

Can you elaborate on why exactly it isn't supported. I'm primarily here because I'm trying to better understand Rails autoloading.

[thedarkone]

Can you elaborate on why exactly it isn't supported.

Simplicity of the implementation. AS::Dependencies is the Rails' magical autoloading mechanism, it needs to track const loading and const unloading to do that (obviously). The most straightforward way to code it is to design it to be the only place that is responsible for const loading and const unloading (in other words AS::Dep is the only place where Object.send :remove_const should be happening).

On the other hand you have DescendantsTracker that also takes advantage of AS::Dep (and its tracking of loading/unloading) to achieve infinitely better perf than Object.descendants (look at the code for more info).

I'm primarily here because I'm trying to better understand Rails autoloading.

If that is your goal, sure, but there is source code available for that 😈.