Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@_shield memoization doesn't remove authenticated state for user #24

Open
christiankakesa opened this issue Sep 15, 2018 · 4 comments
Open

Comments

@christiankakesa
Copy link

christiankakesa commented Sep 15, 2018

I use Shield in rack application with Syro. On logout, I suspect that @_shield memoization persist the authenticate state of user.

For now I remove @_shield memoization and it works :
https://github.com/fenicks/forgedtofight.io/blob/master/decks/basic_deck.rb#L9.

Is it safe to do so ?

@soveran
Copy link
Collaborator

soveran commented Sep 16, 2018

I'm suspecting @_shield is not thread safe. I I'll try something later today, because looking at the code @_shield[model] is eliminated on logout, but it could be the case that it is deleted only in one thread. Or maybe I'm talking nonsense, I'll investigate and let you know. Which webserver are you using?

@christiankakesa
Copy link
Author

Good point, I'm using Unicorn (Worker/Multi processes/single thread).
When I try with Webrick it works perfectly.

@soveran
Copy link
Collaborator

soveran commented Sep 17, 2018

Can you try this?

module Shield
private
  class ThreadSafeHash
    def initialize
      @hs = Hash.new
    end

    def hs
      @hs[Thread.current.object_id] ||= Hash.new
    end

    def [](key)
      hs[key]
    end

    def []=(key, value)
      hs[key] = value
    end

    def delete(key)
      hs.delete(key)
    end
  end
end

Then, in authenticated:

def authenticated(model)
  @_shield ||= ThreadSafeHash.new
  @_shield[model] ||= session[model.to_s] && model[session[model.to_s]]
end

@christiankakesa
Copy link
Author

I tryed, but it doesn't fix the problem on logout with Unicorn, works with WEBrick.

It's probably because of workers.
Unicorn launch at least 2 worker and the strategy is round robin (I guess).
It could work with sticky mechanism on workers but it's too heavy.
In this situation we probably need a shared data structure between workers which Rack::Session::NoBrainer is (in my case) but It's a slow solution that serialize @_shield in session.

Don't know if there is a good solution for that ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants