Grape: is Puma + Grape not threadsafe?

Created on 21 Mar 2016  路  8Comments  路  Source: ruby-grape/grape

my online Grape Project change unicorn to puma deploy.

so much users get other users data from api.

develop by Rails 4.2 + puma 3.0.1 + Grape 0.9.0.

some code like as:

base.rb

module V3
  class Base < Grape::API 
    helpers V3Helpers

    include V3::Photos
  end
end

v3_helpers.rb

def current_user
return @current_user if defined? @current_user

@current_user = User.find(params[:uid])
end

photos.rb

module V3
  module Photos
    extend ActiveSupport::Concern

    included do

      resources :photos do

        get do 
          photos = Photo.where(user_id: current_user.id)
          photos.to_json 
        end   

      end

    end
end
end

is my code not threadsafe ?

discuss! needs info

Most helpful comment

this issue is not Grape problem

at last i found active_model_serializers problem, am use not release code, it's use class variables, not threadsafe !

gem 'active_model_serializers', git: 'https://github.com/rails-api/active_model_serializers.git', ref: '2df8804'

active_model_serializers/lib/active_model/serializer.rb

def read_attribute_for_serialization(attr)
  if self.class._serializer_instance_method_defined?(attr)
    send(attr)
  elsif self.class._fragmented
    self.class._fragmented.read_attribute_for_serialization(attr)
  else
    object.read_attribute_for_serialization(attr)
  end
end

chinese user , u can see here: https://ruby-china.org/topics/30188

All 8 comments

Are you worried specifically about @current_user? That's not automatically threadsafe. Grape or Puma won't take locks on that. That said, I am not sure what @current_user actual scope is, it might be executed in the context of a block that belongs to the current executing thread. So you _might_ be ok.

I would verify that by examining what self is within that block in a multithreaded environment under load.

Finally, whatever we find should be documented.

I found it difficult to trace the instantiation of middleware and helpers in Grape's internals, but a simple test of self.object_id from within each of (a) a middleware class invoked by use and (b) a helper class invoked by helpers confirmed that the calling context is that of a different object on each request. This would point to instance variables (like in the above example) being isolated to the request and thus safe to use.

It's possible there are other thread-safety issues I'm missing. Happy to research more if anyone can point me to the relevant code.

Maybe use warden to attach current_user to fix this ?

@wxianfeng have you solved/figure out this issue? I want to use grape with puma in my project,but thread safe is a serious problem.

@MoonShining Just to be clear, we're using Grape successfully with Puma on a large scale production application just fine. There're some questions around thread safety that seem to indicate that there's potential for non-thread-safe behavior, but this issue is still open to make sure we aren't missing anything.

Re-reading this thread I am happy with @joeyAghion's answer and am going to close this.

this issue is not Grape problem

at last i found active_model_serializers problem, am use not release code, it's use class variables, not threadsafe !

gem 'active_model_serializers', git: 'https://github.com/rails-api/active_model_serializers.git', ref: '2df8804'

active_model_serializers/lib/active_model/serializer.rb

def read_attribute_for_serialization(attr)
  if self.class._serializer_instance_method_defined?(attr)
    send(attr)
  elsif self.class._fragmented
    self.class._fragmented.read_attribute_for_serialization(attr)
  else
    object.read_attribute_for_serialization(attr)
  end
end

chinese user , u can see here: https://ruby-china.org/topics/30188

Was this page helpful?
0 / 5 - 0 ratings