Memoization - a potential cause for race conditions between threads.

[yeoenggu]

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]

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]

I would get this in the console. [2017-04-18T23:39:22.116391 #5865] ERROR -- omniauth: (shopify) Authentication failure! invalid_credentials:  Shopify will response with http response with the following message in it. "Oauth error invalid_request: The authorization code was not found or was already used" regards, Eng Guan Yeo On Wed, 26 Apr 2017 at 10:27 PM Dylan Thacker-Smith < mailto:Dylan Thacker-Smith <notifications@github.com> > wrote: a, pre, code, a:link, body { word-wrap: break-word !important; } 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? https://github.com/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? — You are receiving this because you authored the thread. Reply to this email directly, #56 (comment) , or https://github.com/notifications/unsubscribe-auth/AFYyq2QUufFT6WHynl-97D7oPaouGm9Hks5rz1RqgaJpZM4NI5jZ .

[kevinhughes27]

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.