Race condition in localstorage store in multiple tabs

[dschmidt]

Hey,

if my page is active in multiple tabs there seems to be a race condition that makes logging in impossible. When one tab tries to persist data into the local storage it's immediately cleared by some other tab. It's not 100% reproducible with two tabs, sometimes it works, sometimes it doesn't. But the more tabs you open the higher the probability is to trigger this behaviour.

If you can't reproduce let me know how I can build a test case for you, jsbin seems rather hard to work with for this kind of stuff.

Maybe it would help to add a mutex/lock key to the localStorage so it's not cleared while persisting data in another tab?

Thanks in advance,
Dominik

[marcoow]

I think the problem is that the localStorage API triggers the 'storage' event for every single write. When the session updates the store it first clears all elements and then persists all elements again (so it doesn't have to keep track which properties have been added/removed/changed). So every update of the store triggers a whole bunch of calls to removeItem and setItem each of which triggers the 'ember-simple-auth:session-updated' which might lead to the store being cleared (when not all required properties are present yet). BTW most of they applies to the cookie store as well (which doesn't use event but periodically checks the cookies for changes; this check might interfere with an update from another tab as well).

I'd say having a lock mechanism would be great. While coming up with a test case that tests the negative case would be pretty hard I'd say it would be sufficient to have a lock implementation accompanied by a test case that only verifies the positive case (no events are triggered when the lock is present and thus a store update is in progress).

Would be great if you'd come up with a pull request!

[dschmidt]

I will try tomorrow.

[dschmidt]

I'm saying hello to a heisenbug. I can't reproduce it today anymore, although I could do so in almost 100% of the cases yesterday :-(

I was really surprised to see that I could never get isLocked() to be true in my implementation, but I assume this is for the same reason. It's possible that my system had a very high load (or Chromium just went nuts) yesterday and then Chromium was too slow to handle all the events properly, today it's behaving and I can't reproduce anymore (no, not in Firefox either).

Any ideas how to approach this beast?

[marcoow]

Hm, I would probably follow a more pragmatic approach and just assume that it can happen (without actually arriving it ;) and simply implement some tests that validate that the locking mechanism prevents the race condition from actually happening.

Also would be cool if you'd base your stuff on the 0.2.0 branch as that changes quite sth. wrt testing etc.

[dschmidt]

So the good news is: one of my team members can reproducibly achieve the race condition on our staging area, but he can't on my test server. I'm on the right track at least. Will rebase onto the 0.2.0 branch, I hope I don't need to change too much of the other code that I wrote before I discovered the 0.2.0 branch. Let's see.

P.S.: Thanks for being so responsive, upstreaming is fun this way

[marcoow]

Cool; don't think you need to change much code, mostly the tests changed as I'm (in the process of) moving from QUnit to Mocha as I found the QUnit tests to be too big/complex and repetitive.

[dschmidt]

Are you on IRC/Jabber for a few quick questions? My name is domme on Freenode.

[marcoow]

don't really use any of those, let's chat here: https://simplabs.campfirenow.com/4711f

[dschmidt]

Superseded by PR.