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

Memoization - a potential cause for race conditions between threads. #56

Open
yeoenggu opened this issue Apr 26, 2017 · 3 comments
Open

Comments

@yeoenggu
Copy link

In OmniAuth/Strategies/Shopify.rb - build_access_token method uses memoization which may cause a potential race condition. Refer to https://bearmetal.eu/theden/how-do-i-know-whether-my-rails-app-is-thread-safe-or-not/

  1. Supposed thread A has successfully build the token but yet to assign it to the instance variable.
  2. Thread B access the variable and since it is not yet set, it tries to build the token using same params. This result in error since code can only be used once.

I did have this bug occasionally where I received an error from Shopify. Unfortunately, I could not replicate it.

@dylanahsmith
Copy link
Contributor

build_access_token overrides an upstream method that doesn't do any memoization, so I'm not sure why it would be unsafe to call more than once

This result in error since code can only be used once.

What error does this actually result in?

@EiNSTeiN- looks like he is referring to this method override that you added in #34. Why was that needed? Was that just a performance optimization?

@yeoenggu
Copy link
Author

yeoenggu commented Apr 26, 2017 via email

@kevinhughes27
Copy link

I've investigated auth errors that we thought may be related to race conditions before. More from the browser side of things but this may be worth looking into is all I wanted to add.

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

3 participants