Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Atomic compare-and-swap operation? #56

Open
billiegoose opened this issue Mar 12, 2019 · 5 comments
Open

Atomic compare-and-swap operation? #56

billiegoose opened this issue Mar 12, 2019 · 5 comments
Labels
addition A proposed new capability api A proposal for changing the API surface

Comments

@billiegoose
Copy link

I recently ran into an issue with race conditions between the main thread and a worker thread clobbering each other's writes using @jakearchibald 's idb-keyval, and decided I needed a function like:

let oldValue: any = await idb.swap(
  key: string,
  expect: (oldValue: any) => boolean,
  value: any
);

as a primitive in order to implement a mutex. Unfortunately, even after forking idb-keyval and hacking on it for a bit, I wasn't able to code such a function correctly, AFAICT. 😭

I know that thread-safety is probably beyond the scope of this module (see #5) but I think it would be nice if a future version could add support for atomic operations.

@jakearchibald
Copy link

Off-topic, but I think this would work as an addition to idb-keyval:

export function update(key: IDBValidKey, updater: (val: any) => any, store = getDefaultStore()): Promise<void> {
  return store._withIDBStore('readwrite', store => {
    const req = store.get(key);
    req.onsuccess = () => {
      store.put(updater(req.result), key);
    };
  });
}

@billiegoose
Copy link
Author

re: offtopic - Oh that's so much simpler than what I was doing! I was messing around with trying to abort the transaction.

@domenic
Copy link
Collaborator

domenic commented Mar 13, 2019

This is very tempting to me, but in the past our storage folks have said that we should not encroach too much on IndexedDB's territory, instead referring people to the backingStore API. I can appreciate the clean separation. WDYT, @asutherland @pwnall and others?

@asutherland
Copy link

For the explicit request: I don't think compare-and-swap is appropriate semantics to layer on top of transactions. As I understand the compare-and-swap primitive it is a general low-level multi-threading primitive and in the event of failure values are re-computed and the operation re-attempted. It seems more appropriate to use transactions to address the concurrency issue, as Jake has proposed.

For implementing something like update, I'm open to the idea, but I think it's important to figure out the overall plan as it relates to making IndexedDB more friendly. If we're going to clean up the IndexedDB API to directly support promises and async/await as covered by w3c/IndexedDB#42, that might be the better avenue. For example, the proposed update helper above does not provide the ability to atomically manipulate 2 keys in the same transaction, which leads to foot-guns of its own. My current gut feel is that we should first try and standardize promises and related convenience helpers within the IndexedDB API before growing kv-storage into IDB's territory.

@pwnall
Copy link

pwnall commented Mar 14, 2019

compare-and-swap seems like a specific case of read-modify-write.

If we do add a primitive, I'd prefer that it's readModifyWrite (or map?). It would start a txn, read a key, call a function with the key's value as an input, then write the function's return value. The function has to be synchronous due to IndexedDB limitations.

I've written readModifyWrite in an IndexedDB wrapper before, so I'm not completely against it. Would like to see a use case though. The mutex problem described in the original post should be solved using the Web Locks API.

@domenic domenic added addition A proposed new capability api A proposal for changing the API surface labels Mar 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
addition A proposed new capability api A proposal for changing the API surface
Projects
None yet
Development

No branches or pull requests

5 participants