Hey guys
http://dev.rubyonrails.org/ticket/11540
I've written a patch for a race condition I found in
ActionView::TemplateHandlers::Compilable. And I'm fishing for +1s 
Actually I'd just like some more people to look over it and leave
feedback.
I'd really like Rails to be thread safe and I totally think it's a doable feat. I'd like to help out as much as I can so if anyone knows of any problem areas please point me towards them.
I'd really like Rails to be thread safe and I totally think it's a doable feat. I'd like to help out as much as I can so if anyone knows of any problem areas please point me towards them.
I think it's an achievable feat too, but I'm a little hesitant to just whack mutexes everywhere we find a race condition. Instead I'd like to think a little more about the changes which are needed to make the thing work and rejig the internals to make the task simpler.
I'd suggest a candidate first-task is tidying up the ActiveRecord allow_concurrency stuff to use a connection pool rather than blindly establishing a connection per thread. This would require some pretty heavy-duty refactorings in connection_specification, but it should be reasonably straightforward. If anyone's interested in starting with this, I'd be happy to collaborate and host a thread-safety branch on my github account as a staging area before we include it for 'whatever comes after 2.1'.
I don't know if I can start on it right away, but I'm very interested in helping with this. In particular, the establish_connection stuff and the way connections are cached is not very friendly right now to Java appservers where the connection pooling usually happens underneath the covers. If possible I'd like to find a solution that includes the possibility of opening and closing connections around each request and/or transaction. Opening connections in the appserver is cheap because it's just checking out/returning the connection to the pool. Perhaps any work toward that goal can use a similar abstraction -- then I can wholesale replace Rails' own connection pool with one that connects directly to the Java connection pool.
Cheers, /Nick
I don't know if I can start on it right away, but I'm very interested in helping with this. In particular, the establish_connection stuff and the way connections are cached is not very friendly right now to Java appservers where the connection pooling usually happens underneath the covers. If possible I'd like to find a solution that includes the possibility of opening and closing connections around each request and/or transaction. Opening connections in the appserver is cheap because it's just checking out/returning the connection to the pool. Perhaps any work toward that goal can use a similar abstraction -- then I can wholesale replace Rails' own connection pool with one that connects directly to the Java connection pool.
I spoke about this a little in edinburgh, but connection_specification is just screaming for some kind of class to manage all that stuff instead of a bunch of class methods with conditionals. Even without any functional changes that code smells and should be made much easier to customise. Not to sound too 'enterprise' but a simple 'connection factory' would make most people's lives that much easier ;).
I have my thread safe branch setup already. I'm just toying around w/ some ideas right now. I'll actually start working on it when this Google Code thing kicks off.
I think it's an achievable feat too, but I'm a little hesitant to just whack mutexes everywhere we find a race condition. Instead I'd like to think a little more about the changes which are needed to make the thing work and rejig the internals to make the task simpler.
I agree, mutexes are not the solution for every threading problem, but I believe for some problems they are the best (most pragmatic) solution. There is a non mutex solution to this race condition which is to use a unique method name for every combination of template-name/ local-assings (currently the method name is only based on the template name). To me this is more complex and less elegant than a mutex.
There may be a larger reworking of template compilation that fixes this race condition. But I don't see an issue with fixing it with a mutex until someone takes on that task. I'd love to hear if you have a different or better solution to this though.
I'd suggest a candidate first-task is tidying up the ActiveRecord allow_concurrency stuff to use a connection pool rather than blindly establishing a connection per thread. This would require some pretty
On ActiveRecord, I think it's great that people want to improve it.
But personally I want to focus on the parts of rails which are not yet
thread safe. AR is at least somewhat thread-safe already. Of course
I have no problem with anyone else working on it 
I agree, mutexes are not the solution for every threading problem, but I believe for some problems they are the best (most pragmatic) solution. There is a non mutex solution to this race condition which is to use a unique method name for every combination of template-name/ local-assings (currently the method name is only based on the template name). To me this is more complex and less elegant than a mutex.
There may be a larger reworking of template compilation that fixes this race condition. But I don't see an issue with fixing it with a mutex until someone takes on that task. I'd love to hear if you have a different or better solution to this though.
> I'd suggest a candidate first-task is tidying up the ActiveRecord > allow_concurrency stuff to use a connection pool rather than blindly > establishing a connection per thread. This would require some pretty
On ActiveRecord, I think it's great that people want to improve it. But personally I want to focus on the parts of rails which are not yet thread safe. AR is at least somewhat thread-safe already. Of course I have no problem with anyone else working on it
Rails should compile and cache all the templates on boot. This means before any dispatch threads are even run.
BTW, just adding mutexes in patch by patch isn't really going work.
There is already a mutex that wraps the dispatch by mongrel and
another that AP wraps the dispatch w/. (We need to drop the one in the
mongrel handler). I suggest working on this stuff in a different
branch (aka my thread_safe branch until we've got it done.
I think the bigger problem is getting your app to be threadsafe. We should look into adding some helpful development features to find non threadsafe code. I added a feature to my branch that warns you if you try to modify class variables through a caccessor if it is not safe. I made a "Thread.current.safe?" that checks if you are 1) In a thread and 2) not inside a mutex. (all highly experimental!)
Rails should compile and cache all the templates on boot. This means before any dispatch threads are even run.
Same thing I talked about in IRC. The challenge here is to deal with partial locals without falling back to method_missing hack and not screw up the performance, in case anyone is up for it .
BTW, just adding mutexes in patch by patch isn't really going work. There is already a mutex that wraps the dispatch by mongrel and another that AP wraps the dispatch w/. (We need to drop the one in the mongrel handler). I suggest working on this stuff in a different branch (aka my thread_safe branch
Agree.
Rails should compile and cache all the templates on boot. This means
before any dispatch threads are even run.
I think this is a great goal although it not necessary to solve the race condition in template compilation.
BTW, just adding mutexes in patch by patch isnât really going work.
There is already a mutex that wraps the dispatch by mongrel and
another that AP wraps the dispatch w/. (We need to drop the one in the
mongrel handler). I suggest working on this stuff in a different
branch (aka my thread_safe branch
I would agree on dropping mongrel mutex since AP protects the dispatch already, except for⌠do some older versions of rails not have this mutex? In which case weâd have a problem.
What I was planning on doing is attacking thread-safety on a case by case basis. We can leave the AP dispatch mutex until all known thread safety problems are solved, at which point we can remove it. Is there an issue with having redundant mutexes until that point?
Sorry sent last email too early!
BTW, just adding mutexes in patch by patch isnât really going work.
There is already a mutex that wraps the dispatch by mongrel and
another that AP wraps the dispatch w/. (We need to drop the one in the
mongrel handler). I suggest working on this stuff in a different
branch (aka my thread_safe branch
Iâm totally cool with working on stuff in your branch.
I think the bigger problem is getting your app to be threadsafe. We
should look into adding some helpful development features to find non
threadsafe code. I added a feature to my branch that warns you if you
try to modify class variables through a caccessor if it is not safe. I
made a âThread.current.safe?â that checks if you are 1) In a thread
and 2) not inside a mutex. (all highly experimental!)
I think having stuff which helps developers write thread-safe code is great although I would disagree it is a âbiggerâ problem. There is absolutely no point for a rails developer to write thread safe code at this point since rails itself isnât thread safe. But hopefully that wonât be the case soon!
Rails should compile and cache all the templates on boot. This means
before any dispatch threads are even run.
Also I forgot to add, I think inline templates would make this an especially hard goalâŚ
There may be a larger reworking of template compilation that fixes this race condition. But I don't see an issue with fixing it with a mutex until someone takes on that task. I'd love to hear if you have a different or better solution to this though.
I'm not suggesting that there's a magical rewrite which avoids the need for a mutex altogether. But it would be nice to refactor the compilation code so that we have a single method with the mutex around it, and perhaps look at other race conditions in the same space.
I think josh's thread-safe branch is definitely a place to start playing around.