On Wed, May 04, 2011 at 06:32:52PM -0700, Ketan Padegaonkar wrote: > Hey, > > We hit a bug today because Arel::Visitors::ToSql is not threadsafe. > Here is what is happening: > > Arel::Visitors::ENGINE_VISITORS is a cache of visitors instances. > These instances are not inherently threadsafe because it contains > state '@last_column', '@connection' that is shared between threads. > > The other variables '@pool', '@quoted_tables' and '@quoted_columns' > seem just fine. I'd like to propose a patch > (https://github.com/ketan/arel/commit/52dc69f5bebebb75b7b3d7bd42e839f2db54b88a) > that fixes this using ruby's threadlocal for the last_column and > connection instance variables. Would be great if this was pulled into > master. > > Any other thoughts or other recommendations on approaching this?
Sorry to take so long to reply to this. Google groups was bouncing my emails for some reason. I think the patch is fine for now. I'd like to eventually cache the visitor on each connection (as there should only be one connection / thread) and adjust the AST such that "last_column" is no longer needed. I'll apply these patches and make a release. Thank you! -- Aaron Patterson http://tenderlovemaking.com/
pgpwdyZbOf5wL.pgp
Description: PGP signature
