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

Thread.Current storage under unicorn/thin #34

Closed
toxaq opened this issue Feb 18, 2013 · 8 comments
Closed

Thread.Current storage under unicorn/thin #34

toxaq opened this issue Feb 18, 2013 · 8 comments

Comments

@toxaq
Copy link

toxaq commented Feb 18, 2013

When using thin or unicorn Thread.Current isn't guaranteed to be cleared between requests, so you could end up having a tenant set when you don't want it set. As per this SO Question

There seems to be two solutions here:

  1. Add a note to the readme to create a before_filter that sets the current_tenant to nil if you aren't explicitly setting a tenant for each request
  2. Use something like RequestStore as referenced in this rails issue thread

Thoughts?

@jfrux
Copy link

jfrux commented Feb 19, 2013

Is this related to race conditions?
Could it be possible for a record to get saved to an incorrect tenant if the user requests a page on a different tenant right before the save takes place...?

@toxaq
Copy link
Author

toxaq commented Feb 19, 2013

Only if you aren't guaranteed to set a tenant in each request. In my case I had two namespaced sections of the site, one which guaranteed a tenant was set and one which didn't. The section not setting a tenant would often throw errors because a tenant was still set when it shouldn't have been (and hadn't been during that request). I now explicitly set the tenant to nil on the non-tenant section.

@ErwinM
Copy link
Owner

ErwinM commented Feb 20, 2013

I read the SO post. Interesting stuff.

I'm thinking of clearing the Thread.current[:current_tenant] explicitly, but I'm having trouble seeing all the use cases.

@toxaq In the app you are talking about in your comment: on the section without the tenant set, is the code still passing through the tenant assignment in application_controller (set_tenant_by_subdomain or using the before_filter approach) or is it somehow skipping it?

@toxaq
Copy link
Author

toxaq commented Feb 21, 2013

Yeah, I'm using the before filter approach. I only set it if required, so it effectively skipped the setting. Setting to nil explicitly works fine (my fix).

@ErwinM
Copy link
Owner

ErwinM commented Feb 21, 2013

I've switched the gem (master) to using request_store instead of Thread.current. This should ensure a clean slate for each request.

@toxaq If you could find the time to test if this indeed fixes the issues you were experiencing (without your later fixes), that would be much appreciated!

@ErwinM ErwinM closed this as completed Feb 23, 2013
@toxaq
Copy link
Author

toxaq commented Mar 2, 2013

Just caught up on this, will try a test this week and see how I go. Thanks for the update!

@swordfish444
Copy link

@toxaq Were you ever able to determine the issue and what resolved it?

@toxaq
Copy link
Author

toxaq commented Mar 27, 2018

@swordfish444 it's a long time ago now but I think Erwin's change to request_store resolved this but explicitly setting on every request in a before_filter also worked adequately.

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

4 participants