Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autoloading and Rails #26

Closed
pmorton opened this issue Apr 26, 2016 · 5 comments
Closed

Autoloading and Rails #26

pmorton opened this issue Apr 26, 2016 · 5 comments

Comments

@pmorton
Copy link
Contributor

pmorton commented Apr 26, 2016

We are getting a NameError: uninitialized constant Runtime exception from jmespath at random intervals from threaded calls. The app is rails 4.2 with puma running with 2 clustered mode workers with 16 threads. This generally does not make sense because the constant is defined. After much research, it seems that this is probably an issue with the use of autoload. In fact I have successfully resolved this issue in a few other GEMS by either using straight require or plugging into rails autoloading.

https://bugs.ruby-lang.org/issues/921 confirms that for 1.9.3 and older, autoload is not thread safe while 2.x is. Further @headius seems to indicate that autoload is only thread safe is the same autoload mechanism is used across the board. headius/thread_safe#15 (comment). So here is where I believe that issue to be: Rails uses it's own autoloading systems, while this gem uses another. There is some race condition that is causing the Runtime class to only be partially loaded when it is called.

To address this issue we could take one of a couple of approaches

Dump autoloading and use require. This would make the gem slower to load, but this should be trivial since the gem is pretty small.

Hook into the rails autoloading system. Each file using autoloads would add the following to the class
extend ActiveSupport::Autoload if defined?(ActiveSupport::Autoload). The net result is that the activesupport autoloader which is used by rails would also be used by this gem. Doing this would allow lazy loading for ruby files in development and eager loading in production.

I believe that 1 is the simplest with very little performance tradeoff, but if load performance is important to you option 2 would work as well. Let me know what you think. I am happy to code this up.

@pmorton
Copy link
Contributor Author

pmorton commented Apr 26, 2016

@trevorrowe Since this is actually happening for us in the AWS ruby gem, I thought you would be interested.

@pmorton
Copy link
Contributor Author

pmorton commented Apr 27, 2016

Well - for this particular gem, the issue could be http://urbanautomaton.com/blog/2013/08/27/rails-autoloading-hell/
In either case, avoiding autologous would solve the issue.

@trevorrowe
Copy link
Contributor

Given the small size of the library, I am in favor if removing the use of autoload. On a side note, we are investigating modularizing the 70+ services into their own gems. The end goal is to possibly remove the need for autoload in aws-sdk. Still in a very early stage considering this.

@pmorton
Copy link
Contributor Author

pmorton commented Apr 28, 2016

Perfect. I will get in a pr.

@trevorrowe
Copy link
Contributor

Merged #31, this will go out with the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants