34

I'm curious about the possibility of damaging localStorage entry by overwriting it in two browser tabs simultaneously. Should I create a mutex for local storage?
I was already thinking of such pseudo-class:

LocalStorageMan.prototype.v = LocalStorageMan.prototype.value = function(name, val) {
  //Set inner value
  this.data[name] = val;
  //Delay any changes if the local storage is being changed
  if(localStorage[this.name+"__mutex"]==1) {
    setTimeout(function() {this.v(name, val);}, 1);
    return null;  //Very good point @Lightness Races in Orbit 
  }
  //Lock the mutext to prevent overwriting
  localStorage[this.name+"__mutex"] = 1;
  //Save serialized data
  localStorage[this.name] = this.serializeData;
  //Allow usage from another tabs
  localStorage[this.name+"__mutex"] = 0;
}

The function above implies local storage manager that is managing one specific key of the local storage - localStorage["test"] for example. I want to use this for greasomonkey userscripts where avoiding conlicts is a priority.

Tomáš Zato
  • 50,171
  • 52
  • 268
  • 778
  • 2
    Yes it is thread safe - and it also fires change events to all other threads when a single tab makes a modification so you don't need to poll it manually. – Benjamin Gruenbaum Feb 24 '14 at 23:08
  • 5
    Your custom mutex implementation isn't thread-safe. – zerkms Feb 24 '14 at 23:10
  • 1
    Also what @zerkms said, your locking isn't atomic so it's not thread problematic. What if I get two threads right after the `if` before the mutex value was assigned? They'd both reassign the data. – Benjamin Gruenbaum Feb 24 '14 at 23:10
  • @BenjaminGruenbaum About the "not safety" note: I thought about that of course - but I see no way to go around that - so I just decided to make it "*safer*". Generally I assumed that assigning INT is faster than assigning whole string. – Tomáš Zato Feb 24 '14 at 23:15
  • You also don't `return` after setting the timeout. – Lightness Races in Orbit Feb 24 '14 at 23:16
  • @TomášZato you have uncovered the fact that it is impossible to implement mutexes from user land code without operating system support. You should be proud of the fact you figured this out on your own, not joking here at all. That said, it is what it is. – Benjamin Gruenbaum Feb 24 '14 at 23:16
  • @LightnessRacesinOrbit I return `null` implicitly. But I agree that it's a good habit to return the value being set. – Tomáš Zato Feb 24 '14 at 23:16
  • @TomášZato what Lightness meant is that you proceed to "lock", assign and unlock _regardless_ of the state. You don't have an `else` there or an _early_ return. Also, functions in JS return `undefined` implicitly and not null. – Benjamin Gruenbaum Feb 24 '14 at 23:17
  • @TomášZato: I believe you missed my point. Even pretending that the mutex logic is atomic (which, as discussed above, is not the case; but, as also mentioned above, that's not really your fault), after you find that the mutex is already locked, you set a timeout to try your function again after a period of time, then happily proceed to perform all the following logic anyway. You probably meant to `return` directly after the `setTimeout`, or to enclose all the following statements in an `else`. – Lightness Races in Orbit Feb 24 '14 at 23:17
  • Yeah, I really missed it @LightnessRacesinOrbit. I've edited my question. The code was written in hurry as I had suspition it's useless. – Tomáš Zato Feb 24 '14 at 23:19
  • @BenjaminGruenbaum I'm currently searching for the events you talked about. Anything you'd recommend google search results aside? – Tomáš Zato Feb 24 '14 at 23:22
  • @TomášZato Probably for `StorageEvent` , but if you wanted to learn about those you can just ask :) Here is a demo http://html5demos.com/storage-events and here is a reference with a demo http://msdn.microsoft.com/en-us/library/ie/ff974349(v=vs.85).aspx – Benjamin Gruenbaum Feb 24 '14 at 23:23
  • The first result for `localstorage events` is pretty clear – Lightness Races in Orbit Feb 24 '14 at 23:24

2 Answers2

41

Yes, it is thread safe. However, your code isn't atomic and that's your problem there. I'll get to thread safety of localStorage but first, how to fix your problem.

Both tabs can pass the if check together and write to the item overwriting each other. The correct way to handle this problem is using StorageEvents.

These let you notify other windows when a key has changed in localStorage, effectively solving the problem for you in a built in message passing safe way. Here is a nice read about them. Let's give an example:

// tab 1
localStorage.setItem("Foo","Bar");

// tab 2
window.addEventListener("storage",function(e){
    alert("StorageChanged!"); // this will run when the localStorage is changed
});

Now, what I promised about thread safety :)

As I like - let's observe this from two angles - from the specification and using the implementation.

The specification

Let's show it's thread safe by specification.

If we check the specification of Web Storage we can see that it specifically notes:

Because of the use of the storage mutex, multiple browsing contexts will be able to access the local storage areas simultaneously in such a manner that scripts cannot detect any concurrent script execution.

Thus, the length attribute of a Storage object, and the value of the various properties of that object, cannot change while a script is executing, other than in a way that is predictable by the script itself.

It even elaborates further:

Whenever the properties of a localStorage attribute's Storage object are to be examined, returned, set, or deleted, whether as part of a direct property access, when checking for the presence of a property, during property enumeration, when determining the number of properties present, or as part of the execution of any of the methods or attributes defined on the Storage interface, the user agent must first obtain the storage mutex.

Emphasis mine. It also notes that some implementors don't like this as a note.

In practice

Let's show it's thread safe in implementation.

Choosing a random browser, I chose WebKit (because I didn't know where that code is located there before). If we check at WebKit's implementation of Storage we can see that it has its fare share of mutexes.

Let's take it from the start. When you call setItem or assign, this happens:

void Storage::setItem(const String& key, const String& value, ExceptionCode& ec)
{
    if (!m_storageArea->canAccessStorage(m_frame)) {
        ec = SECURITY_ERR;
        return;
    }

    if (isDisabledByPrivateBrowsing()) {
        ec = QUOTA_EXCEEDED_ERR;
        return;
    }

    bool quotaException = false;
    m_storageArea->setItem(m_frame, key, value, quotaException);

    if (quotaException)
        ec = QUOTA_EXCEEDED_ERR;
}

Next, this happens in StorageArea:

void StorageAreaImpl::setItem(Frame* sourceFrame, const String& key, const String& value, bool& quotaException)
{
    ASSERT(!m_isShutdown);
    ASSERT(!value.isNull());
    blockUntilImportComplete();

    String oldValue;
    RefPtr<StorageMap> newMap = m_storageMap->setItem(key, value, oldValue, quotaException);
    if (newMap)
        m_storageMap = newMap.release();

    if (quotaException)
        return;

    if (oldValue == value)
        return;

    if (m_storageAreaSync)
        m_storageAreaSync->scheduleItemForSync(key, value);

    dispatchStorageEvent(key, oldValue, value, sourceFrame);
}

Note that blockUntilImportComplete here. Let's look at that:

void StorageAreaSync::blockUntilImportComplete()
{
    ASSERT(isMainThread());

    // Fast path.  We set m_storageArea to 0 only after m_importComplete being true.
    if (!m_storageArea)
        return;

    MutexLocker locker(m_importLock);
    while (!m_importComplete)
        m_importCondition.wait(m_importLock);
    m_storageArea = 0;
}

They also went as far as add a nice note:

// FIXME: In the future, we should allow use of StorageAreas while it's importing (when safe to do so).
// Blocking everything until the import is complete is by far the simplest and safest thing to do, but
// there is certainly room for safe optimization: Key/length will never be able to make use of such an
// optimization (since the order of iteration can change as items are being added). Get can return any
// item currently in the map. Get/remove can work whether or not it's in the map, but we'll need a list
// of items the import should not overwrite. Clear can also work, but it'll need to kill the import
// job first.

Explaining this works, but it can be more efficient.

Community
  • 1
  • 1
Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
  • 1
    Reading this again, this wasn't explained - the locking I mentioned here is when using localStorage for the first time, the actual thing synchronizing the different tabs together is SQLite which takes care of concurrency here. – Benjamin Gruenbaum Feb 24 '14 at 23:56
  • 1
    Unfortunately not all browsers follow the spec. Yes, it seems that Chrome has fixed it at some point, but I checked as recently as January 2014, and it was still an issue in IE11. – balpha Feb 25 '14 at 07:05
  • @balpha Thanks for the comment! Is this the bug you refer to http://connect.microsoft.com/IE/feedback/details/812563/ie-11-local-storage-synchronization-issues ? It should not be a problem when you use storage events rather than polling for changes. Here is a related issue http://stackoverflow.com/questions/20565508/how-to-work-around-ie11-localstorage-events-firing-twice-or-not-at-all-in-iframe . This is an iframe problem IE problem. This was fixed in WebKit (not in Chrome directly), Chrome however (using blink, a fork of WebKit) also has this working. – Benjamin Gruenbaum Feb 25 '14 at 07:26
  • 1
    No, I was referring to being able to read and write the LS synchronously and being sure that what I overwrite is exactly what I read previously. I absolutely agree with you on the `storageEvent` though (I recently gave [a talk](http://thewcdc.net/conf/session/11) on it). When I wrote http://balpha.de/2012/03/javascript-concurrency-and-locking-the-html5-localstorage/, the storage event wasn't yet complete and reliable in current browsers, but these days that's really not an issue anymore. – balpha Feb 25 '14 at 08:11
  • @balpha cool, do you have the slides or a recording of that talk? – Benjamin Gruenbaum Feb 25 '14 at 08:38
  • 1
    @BenjaminGruenbaum: Suppose I open multiple windows/tabs, and I want exactly one of them set key `localStorage.setItem('key', randomValue)`. How can I do this? – Jack Cood Jun 18 '14 at 20:28
  • @JackCood The first one sets it, that emits a storage event and everyone else listens to it and known not to set it. – Benjamin Gruenbaum Jun 18 '14 at 20:59
  • @BenjaminGruenbaum: this is my sample control flow: http://jsbin.com/neyuyafa/2/edit. How can I make it right as you said? – Jack Cood Jun 19 '14 at 03:43
  • @BenjaminGruenbaum: Or can you give me other convenient way to do this? I thought about this problem many times but it seems quite complicated for me. – Jack Cood Jun 19 '14 at 11:27
  • 10
    I do not understand how using storageEvent could help with concurrency problem. Could anybody bring an example of code which increments a value in localStorage and does not have a concurrency problem because of using storageEvents? – user1608790 Jan 28 '15 at 11:35
  • 2
    @user1608790 such an example does not exist. – Benjamin Gruenbaum Jun 20 '16 at 09:18
  • 3
    ...and can't exist? Javascript is only working one function at a time for a particular page. If I'm not mistaking you can't extract then store an incremented version of the extracted value and be sure that between the two steps (well more because the incrementation is one step of its own) another tab has not changed its value, and you can be losing at least 1 incrementation. – Micaël Félix Jun 23 '16 at 12:59
  • 1
    That is correct, note hat this is not a problem with IndexedDB which has transactions. You can spawn a service worker and synchronize localStorage through it. – Benjamin Gruenbaum Jun 23 '16 at 16:32
  • 1
    If I follow the link to the [HTML5 specification’s talk about the Storage Mutex](https://www.w3.org/TR/html5/webappapis.html#storage-mutex), it seems to say that user agents are allowed to ignore it but that doing so will cause web pages to run incorrectly. The CXX code you showed doesn’t seem to show a storage mutex being grabbed and scheduled for release on the next tick, and I thought that Chrome was one of the browsers where different tabs actually ran independent JavaScript engines. If we can assume Storage Mutex, then people wouldn’t be implementing mutexes on top of it. – binki Dec 14 '16 at 23:05
  • Worth mentioning that Google are working on a "proper" solution for synchronizing storage between tabs and there is ongoing chromium work which _may_ end up making it to the spec – Benjamin Gruenbaum Oct 17 '17 at 11:32
4

No, it's not. Mutex was removed from the spec, and this warning was added instead:

The localStorage getter provides access to shared state. This specification does not define the interaction with other browsing contexts in a multiprocess user agent, and authors are encouraged to assume that there is no locking mechanism. A site could, for instance, try to read the value of a key, increment its value, then write it back out, using the new value as a unique identifier for the session; if the site does this twice in two different browser windows at the same time, it might end up using the same "unique" identifier for both sessions, with potentially disastrous effects.

See HTML Spec: 12 Web storage

LEW21
  • 161
  • 4