This project is archived and is in readonly mode.

#3457 open

ActionController::Caching::Sweeper controller instance is not thread safe

Reported by blythe | November 4th, 2009 @ 09:11 PM

Since Sweeper is a singleton(derived from ActiveRecord::Observer or ActiveModel::Observer edge) it seems that multiple threads could modify the single controller instance set and cleared in the around_filter.

  class Sweeper < ActiveRecord::Observer #:nodoc:
    attr_accessor :controller

    def before(controller)
      self.controller = controller
      callback(:before) if controller.perform_caching

    def after(controller)
      callback(:after) if controller.perform_caching
      # Clean up, so that the controller can be collected after this request
      self.controller = nil

Would using Thread.current be preferable?

Comments and changes to this ticket

  • Timothy Jones
  • Michael Rykov

    Michael Rykov April 7th, 2010 @ 09:07 PM

    This often yields to the following errors when two requests overlap on the sweeper

    NoMethodError (undefined method `controller_name' for nil:NilClass):
      ...actionpack-2.3.5/lib/action_controller/caching/sweeper.rb:32:in `callback'
      ...actionpack-2.3.5/lib/action_controller/caching/sweeper.rb:14:in `after'
      ...actionpack-2.3.5/lib/action_controller/filters.rb:208:in `around_proc'


    class ActionController::Caching::Sweeper
      def controller
      def controller=(c)
        Thread.current[:"#{self}_controller"] = c
  • Ryan Bigg

    Ryan Bigg December 15th, 2010 @ 04:57 AM

    • State changed from “new” to “open”
    • Importance changed from “” to “”
  • Greg Hazel

    Greg Hazel December 15th, 2010 @ 05:01 AM


    Ran in to this (several times a second...) when trying to use Rails with config.threadsafe!

  • Michael Koziarski

    Michael Koziarski December 20th, 2010 @ 02:37 AM

    I'd take this as fix for 2-3-stable and possibly 3-0-stable. but for master we should probably do this right and have a sweeper instance per controller rather than a weird 'instance variable backed by a thread local'. There's a reason why we have a controller per request rather than request, session, etc all being in Thread.current.

    Care to take a stab at that larger refactoring?

  • Matt D

    Matt D February 9th, 2011 @ 10:31 PM

    Running into this issue regularly in production. +1

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>