Safety of Thread.current[] usage in rails

Giuseppe picture Giuseppe · Oct 25, 2011 · Viewed 23.3k times · Source

I keep getting conflicting opinions on the practice of storing information in the Thread.current hash (e.g., the current_user, the current subdomain, etc.). The technique has been proposed as a way to simplify later processing within the model layer (query scoping, auditing, etc.).

Many consider the practice unacceptable because it breaks the MVC pattern. Others express concerns about reliability/safety of the approach, and my 2-part question focuses on the latter aspect.

  1. Is the Thread.current hash guaranteed to be available and private to one and only one response, throughout its entire cycle?

  2. I understand that a thread, at the end of a response, may well be handed over to other incoming requests, thereby leaking any information stored in Thread.current. Would clearing such information before the end of the response (e.g. by executing Thread.current[:user] = nil from a controller's after_filter) suffice in preventing such security breach?

Thanks! Giuseppe

Answer

Maurício Linhares picture Maurício Linhares · Nov 28, 2011

There is not an specific reason to stay away from thread-local variables, the main issues are:

  • it's harder to test them, as you will have to remember to set the thread-local variables when you're testing out code that uses it
  • classes that use thread locals will need knowledge that these objects are not available to them but inside a thread-local variable and this kind of indirection usually breaks the law of demeter
  • not cleaning up thread-locals might be an issue if your framework reuses threads (the thread-local variable would be already initiated and code that relies on ||= calls to initialize variables might fail

So, while it's not completely out of question to use, the best approach is not to use them, but from time to time you hit a wall where a thread local is going to be the simplest possible solution without changing quite a lot of code and you will have to compromise, have a less than perfect object oriented model with the thread local or changing quite a lot of code to do the same.

So, it's mostly a matter of thinking which is going to be the best solution for your case and if you're really going down the thread-local path, I'd surely advise you to do it with blocks that remember to clean up after they are done, like the following:

around_filter :do_with_current_user

def do_with_current_user
    Thread.current[:current_user] = self.current_user
    begin
        yield
    ensure
        Thread.current[:current_user] = nil
    end      
end

This ensures the thread local variable is cleaned up before being used if this thread is recycled.