Fork me on GitHub

February 2, 2012

Preventing Recursion in Ruby (without Thread.current)

Post moved to

This post is inspired by Preventing Recursion in Ruby.

First a confession: every time I see Thread.current[] I start thinking of ways to eliminate it. Thus I believe there's at least one thing wrong with the approach presented in the post. Let's try rebuilding prevent_recursion :

It seems to serve almost the same purpose and using an object's singleton class just feels right. Please note, that this does not behave exactly the same way compared to the threaded solution - we prevented a method from recursing once (or "forever" if we remove the (class << self) line), however this should most likely suffice.

Of course, I only got "smart" by failing previously, but since than I always think twice over storing anything in Thread.current, which is one of the most abused patterns in Ruby and no matter how uniquely you name those variables it's still a smell. Maybe it's about time thread-safe frameworks, such as Rails, should provide us with an abstraction for safely storing thread local data.

Now, I should probably end this right here - point proven. But since the post mentioned infinite recursion with ActiveRecord::Callbacks I immediately remembered my very own case where I would be of no luck using the above approach. Try the following sample :

A real world scenario would be keeping an array-like index for items in a basket. Each basket has it's items ordered and changing an index of an item "swaps" it with the other item at the given position. One way of doing it, which requires a recursion prevention would be :

Notice how the method gets redefined with a dummy one to prevent recursion.

I'm a huge fan of doing things DRY, on the other hand sometimes it might be contra-productive, especially when it can't cover all cases or is only used once or twice. Not to mention how much easier it would be for a new comer to read a method such as assign_index and understand the trick immediately then having to dive deep into another module's meta-stuff.

UPDATE: As @avdi pointed me out, I realized I'm actually promoting non-thread-safe code. I narrowed my recursion prevention domain to short lived AR instances that are never accessed beyond a single request, however this is clearly not a completely valid assumption to make. Especially since the original prevent_recursion targets a much wider audience.
Notice the difference in behavior if the prevented (thread-safe) instance method or a shared object is accessed by two concurrent threads, Thread.current will guarantee a correct result for each caller, however my instance_eval approach will not. And so the moral of this story seems that a slice of Thread.current[] "pollution" to prevent recursion is inevitable after all.