User sessions invalid after changing password, but only with multiple threads

Question

I'm running into a strange problem with a feature in my Rails 4 + Devise 3.2 application which allows users to change their password via an AJAX POST to the following action, derived from the Devise wiki Allow users to edit their password. It seems that after the user changes their password and after one or more requests later, they are forcible logged out, and will continue to get forced logged out after signing back in.

# POST /update_my_password
def update_my_password
  @user = User.find(current_user.id)
  authorize! :update, @user ## CanCan check here as well

  if @user.valid_password?(params[:old_password])
    @user.password = params[:new_password]
    @user.password_confirmation = params[:new_password_conf]
    if @user.save
      sign_in @user, :bypass => true
      head :no_content
      return
    end
  else
    render :json => { "error_code" => "Incorrect password" }, :status => 401     
    return
  end

  render :json => { :errors => @user.errors }, :status => 422
end

This action actually works fine in development, but it fails in production when I'm running multi-threaded, multi-worker Puma instances. What is appearing to happen is that the user will remain logged in until one of their requests hits a different thread, and then they are logged out as Unauthorized with a 401 response status. The problem does not occur if I run Puma with a single thread and a single worker. The only way I can seem to allow the user the ability to stay logged in again with multiple threads is to restart the server (which is not a solution). This is rather strange, because I thought the session storage configuration I have would have handled it correctly. My config/initializers/session_store.rb file contains the following:

MyApp::Application.config.session_store(ActionDispatch::Session::CacheStore, :expire_after => 3.days)

My production.rb config contains:

config.cache_store = :dalli_store, ENV["MEMCACHE_SERVERS"],
{ 
  :pool_size => (ENV['MEMCACHE_POOL_SIZE'] || 1),
  :compress => true,
  :socket_timeout => 0.75, 
  :socket_max_failures => 3, 
  :socket_failure_delay => 0.1,
  :down_retry_delay => 2.seconds,
  :keepalive => true,
  :failover => true
}

I am booting up puma via bundle exec puma -p $PORT -C ./config/puma.rb. My puma.rb contains:

threads ENV['PUMA_MIN_THREADS'] || 8, ENV['PUMA_MAX_THREADS'] || 16
workers ENV['PUMA_WORKERS'] || 2
preload_app!

on_worker_boot do
  ActiveSupport.on_load(:active_record) do
    config = Rails.application.config.database_configuration[Rails.env]
    config['reaping_frequency'] = ENV['DB_REAP_FREQ'] || 10 # seconds
    config['pool']              = ENV['DB_POOL'] || 16
    ActiveRecord::Base.establish_connection(config)
  end
end

So... what could be going wrong here? How can I update the session across all threads/workers when the password has changed, without restarting the server?

Answer 1

This is a gross, gross solution, but it appeared that the other threads would do ActiveRecord query caching of my User model, and the stale data returned would trigger an authentication failure.

By adapting a technique described in Bypassing ActiveRecord cache, I added the following to my User.rb file:

# this default scope avoids query caching of the user,
# which can be a big problem when multithreaded user password changing
# happens. 
FIXNUM_MAX = (2**(0.size * 8 -2) -1)
default_scope { 
  r = Random.new.rand(FIXNUM_MAX)
  where("? = ?", r,r)
}

I realize this has performance implications that pervade throughout my application, but it seems to be the only way I could get around the issue. I tried overriding many of the devise and warden methods which use this query, but without luck. Perhaps I'll look into filing a bug against devise/warden soon.

Answer 2

Since you're using Dalli as your session store you may be running up against this issue.

Multithreading Dalli

From the page:

"If you use Puma or another threaded app server, as of Dalli 2.7, you can use a pool of Dalli clients with Rails to ensure the Rails.cache singleton does not become a source of thread contention."

Answer 3

I suspect you're seeing that behavior due to the following issues:

• devise defines the current_user helper method using an instance variable getting the value from warden. in lib/devise/controllers/helpers.rb#58. Substitute user for mapping

  def current_#{mapping}
    @current_#{mapping} ||= warden.authenticate(:scope => :#{mapping})
  end
  

Not having run into this myself, this is speculation, but hopefully it's helpful in some way. In a multi-threaded app, each request is routed to a thread which may be keeping the previous value of the current_user around due to caching, either in thread local storage or rack which may track data per thread.

One thread changes the underlying data (the password change), invalidating the previous data. The cached data shared among other threads is not updated, causing later accesses using the stale data to cause the forced logout. One solution might be to flag that the password changed, allowing the other threads to detect that change and handle it gracefully, without a forced logout.

Answer 4

I would suggest that after a user changes their password, log them out and clear their sessions, like so:

  def update_password
    @user = User.find(current_user.id)
    if @user.update(user_params)
      sign_out @user # Let them sign-in again
      reset_session # This might not be needed?
      redirect_to root_path
    else
      render "edit"
    end
  end

I believe your main issue is the way that sign_in updates the session combined with the multi-threads as you mentioned.