Remove autoload code, it's not thread safe

[tinco]

Hi guys,

We've been running into problems with the autoloads in grape. When running grape in a multithreaded app server it would have multiple concurrent requires which can lead to all sorts of nasty conditions.

I couldn't discover a way to put in thread safe mode so I just removed all the autoloads.

Do you have any other concerns with running Grape in a multithreaded app server?

[tinco]

Woops, pardon me this doesn't work yet, I still have to reorder the dependencies and run the suite. Just submitting the PR as a discussion point for now :)

[tinco]

Alright, specs pass this way :)

[dblock]

Hmm, this will likely break people's workflow. How do other projects deal with this? Can we make Grape thread-safe(r) instead or turn auto-load off in production?

Either way it needs a CHANGELOG entry.

[tinco]

Mr Tenderlove has a blog article about it here:

http://tenderlovemaking.com/2012/06/18/removing-config-threadsafe.html

In ActiveSupport is a module called Autoload, it has a helper method that allows you to call eager_load on modules. What Rails does is including that module and then when config.thread_safe! is set, call it amongst other things.

The module is really tiny, you could opt for that as well, it would disrupt peoples workflows a bit less I suppose.

[dblock]

Grape already requires ActiveSupport, so maybe base it on that?

[tinco]

Definitely, I'll push -f a version that uses that when I get the time, in the mean time I've ran into a thread safety bug in activerecord as well, so enabling multithreading is fun times all around for us ;)

[dm1try]

AFAIK, autoload is thread-safe in CRuby 2.0+. Am I wrong?

[tinco]

It is yes, but only in the sense that Ruby won't load the same file twice in the same thread. Race conditions and deadlocks might still occur. In any case, we are still locked on 1.9 for at least a couple of months. If you wanna go for just 2+ support then that's ok as well, we'll just keep using this fork in the meanwhile.

[dblock]

Thanks @dm1try. I definitely don't want to merge this "as is", I would however merge something that plays nice with ActiveSupport autoload options. Will leave this open.

[tinco]

PR #980 fixes this I believe