Concurrency race condition when using LocalStorage

[mhesler]

My Authenticator is resolving with a multi-value object that include the access_token, refresh_token, .expires, etc.

When a second window is opens the session restore process is initiated which ends up calling setup -> store.replace -> store.clear just before the persist is called again.
In between the clear and persist, the other window storage event is triggered which ends up reading only a portion of the keys that are being persisted by the other window. The sessionDataUpdated event is trigger which begins the same cycle in the other window. This continues until one of them sees a version of the object without the authenticator property. At that point it will clear the store.

Is there a reason we cannot store a JSON.stringify version of the object in a single key? With this approach there is no reason to do a clear first during the replace operation AND we will have a consistent version of the object to compare against and break the cycle.

[seriousben]

Seeing something similar as well. Seems like a major issue.
On Jul 18, 2014 3:12 PM, "Michael Hesler" notifications@github.com wrote:

My Authenticator is resolving with a multi-value object that include the
access_token, refresh_token, .expires, etc.

When a second window is opens the session restore process is initiated
which ends up calling setup -> store.replace -> store.clear just before the
persist is called again.
In between the clear and persist, the other window storage event is
triggered which ends up reading only a portion of the keys that are being
persisted by the other window. The sessionDataUpdated event is trigger
which begins the same cycle in the other window. This continues until one
of them sees a version of the object without the authenticator property. At
that point it will clear the store.

Is there a reason we cannot store a JSON.stringify version of the object
in a single key? With this approach there is no reason to do a clear first
during the replace operation AND we will have a consistent version of the
object to compare against and break the cycle.

Reply to this email directly or view it on GitHub
#254.

[marcoow]

You're right - there certainly is a race condition that so far couldn't be solved (as it's impossible to implement any kind of locking mechanism between a number of tabs/windows that don't know about each other). You're right thought that storing a stringified version in a single key should actually fix this - not sure why nobody thought about that.

[marcoow]

Would be great if you guys could test that branch a bit to verify it fixes the problem. Always hard to reproduce these cases...

[dschmidt]

"Oh god why didn't I have that idea" 👍

[vdveer]

In some rare cases I still see a race condition between tokens. When an application forces a login by dropping a token, after authentication the 'storage' event triggers (session-stores, local-storage.js and validates this._lastData with the newly available storagedata. This event has old and new value available, but this can't be used in all browsers (IE has a bug there), so I am guessing this is why this._lastData has been choosen. But this var potentially has two different values in seperate tabs, in which case bad timing can cause an infinite (until it fails once) authenticator.restore() call with two switching data objects.
In my local project I have resolved this by putting the _lastData object also in a localstorage-record, which as far as I see now fixes this problem for me. I might be overlooking other usages of _lastData, so if you have so spare time, could you take a look at master...vdveer:master ? Thanks @marcoow

[vdveer]

The race condition also triggered because I put a random value on the (non authenticated part) of the session before starting the authentication flow and remove it later on which also triggered a 'storage' event because now all session values use the same localstorage key.

[marcoow]

@vdveer: it's a bit hard to follow what exactly you're describing. Could you create a demo app that shows the bad behavior?