6 ms·
Here's an example of how you might get a race condition in JS: async function deduct(amt) { var balance = await getBalance(); if (balance >
by millstone 7y ago
Here's an example of how you might get a race condition in JS:
async function deduct(amt) {
var balance = await getBalance();
if (balance >= amt)
return await setBalance(balance - amt);
}
One way to resolve this would be with a mutex to protect the balance during the critical section (which is async). What would you suggest instead?
- ghusbands 7y agoMore subtly, here's an example of a hidden race condition in JS: async function totalSize(fol) { const files = await fol.getFiles(); let totalSize = 0; await Promise.all(files.map(async file => { totalSize += await file.getSize(); })); // totalSize is now way too small return totalSize; }
- fourthark 7y agoBecause all the promises are waiting on `fileSize`, right? But do you mean that JS 1. will read `totalSize` then 2. do the asynchronous call, then 3. add and set? Seems like it's ambiguous and JS could just as easily read `totalSize` after the call, and all would be OK. Or is the ordering specified? Thanks for this clever example!
- esprehn 7y agoThe ordering is specified left to right. totalSize += await getSize() becomes totalSize = totalSize + await getSize() So all the map callbacks run one by one, read totalSize as 0, and then suspend waiting for getSize(). Each one then resolves and assigns totalSize to be 0 + the size. The race is what order the getSize() calls return in since only the last one will control the return value. Otherwise the issue isn't a race but just a logical ordering bug. (This isn't super different than doing array[two()] = one() since two will actually run first, so ex. array[i += 1] = i will modify i before assigning the value.) Correct would be to change the body of the map to: const fileSize = await file.getSize(); totalSize += fileSize; or the whole function to: async function totalSize(fol) { const files = await fol.getFiles(); const sizes = await Promise.all(files.map(file => file.getSize())); return sizes.reduce((totalSize, size) => totalSize + size, 0); }
- deleted 7y ago[deleted]
- tlrobinson 7y agoI would have written it in a more functional way: async function totalSize(fol) { const files = await fol.getFiles(); const sizes = await Promise.all(files.map(file => file.getSize())); return sizes.reduce((acc, size) => acc + size); }
- brlewis 7y agoThis is an interesting example because it demonstrates a way to confuse programmers that wasn't previously possible. Thanks for sharing it; it was a gem on an article otherwise full of confused comments. After sharing it at work, someone pointed out that it isn't technically a race condition. The problem isn't caused by operations happening in an unpredictable order. It's unlikely that any of the promises are already resolved, so the lhs is always evaluated first. It's just that the programmer is surprised by the order of operations. The takeaway is unsurprising: that `await` should be a signal to make one think carefully about state change; having `await` right after `+=` should be a big signal. But the fact that an unwary programmer can actually be tripped up is interesting, despite some other comments on this article claiming such trip-ups are inevitable.
- DrJokepu 7y agoI mean, if something like this is behind a promise or an async call, that probably means that there's I/O involved (no point using async to access local in-memory data), which means that the local locking primitive won't be incredibly useful, you're going to have to lock it via whatever mechanism the I/O channel (or some API using the I/O channel) provides, such as file locking or some API call or something.
- ithkuil 7y agoYep, that's the benefit of having to explicitly mark each function explicitly as async. I've heard a lot of people complaining that it's painful, especially during refactorings since it bubbles up, but that's precisely the point: it allows you to know (and choose) whether a function can or cannot be executed interleaved with something else. That said, if new developers just blindly use async/await without understanding what's going on ... well that's another problem. The field is filled with foot-guns, for some definition of guns (and for some definition of foot). The problem with C, or better the problem with pre-emptive multitasking in general, is that even relatively knowledgeable developers were constantly hitting subtle issues with the memory model. Consider the good old trap of efficiently lazy-initializing a Singleton in Java: https://en.m.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java https://en.m.wikipedia.org/wiki/Double-checked_locking#Usage...
- Arelius 7y agoI mean the answer here is an async getBalance and setBalance is the incorrect way to do this, and a mutex won't solve that. if balance is a javascript variable, access shouldn't be provided by async functions. In the case that mutex is a remote resource of some sort (file or network resource) a process local lock won't solve this for you either. It seems to me that the lack of locks make forces software engineers to consider the nature of their data instead of just grabbing for the inappropriate os lock.
- grumpy8 7y ago>> What would you suggest instead? On the front-end, I'd do a call to the server. On the server, I'd use a database transaction for that. But I get your point. The way I deal with race condition in redux is to have a mutex. I.e. While something is being fetched, any call to this command is either ignored or queued up. I use redux-saga for the "race-condition" and general flow, and call the various async functions from within sagas.
- abalone 7y agoHow about an optimistic lock? async function deduct(amt) { var balance = await getBalance(); if (balance >= amt) await setBalance(balance - amt); var newbalance = await getBalance(); if (newbalance != (balance - amt)) { await setBalance(balance + amt); // tell the user the transaction failed... } }
- jtsiskin 7y agoThis is getting off topic, but what would probably be best here is a test and set operation: setBalance(expectedBalance, newBalance)
- tlrobinson 7y agoHow about an `updateBalance(updateFn)` that is protected by a mutex: async function deduct(amt) { await updateBalance(balance => { if (balance >= amt) { return balance - amt; } else { throw new Error("not enough balance"); }) }
- wruza 7y agoAnd they call average C developer less experienced in async. The only correct ‘update balance’ operation is: INSERT INTO CashFlow (date, income, expense) VALUES (:date, :income, 0) Get balance: SELECT sum(income)-sum(expense) AS balance FROM CashFlow What modern js still has to reinvent is in-client synchronizing storage which naturally resolves who waits on what (if at all). This is partially simulated by reactjs now by trading in a developer’s comfort at near zero price. All this “await/promise has clear semantics and we think async” is pointless hope because code should never race with itself. Data should, and the storage must be there to kill associated problems once and forever.
- kristiandupont 7y agoOperational transformations are not the "only correct" way to update an integer and it comes with a number of performance and memory/storage consumption implications. >What modern js still has to reinvent is in-client synchronizing storage which naturally resolves who waits on what (if at all) I don't understand what this means but it makes me curious. What would be an example of this in one of the languages that have already "reinvented" it?
- kaveh_h 7y agoIf the balance is handled in a remote server or location then you should have a remote deduct method which handles the operation in an atomic idempotent manner. If you are handling data locally e.g in a file then you would ideally rely on OS write lock and only store the deltas and never change state. You could then calculate the balance from the log of deductions and insertions. FYI this problem has not anything to do with async or JS.
- spraak 7y agoCan anyone explain this to me, how is it a race condition?
- pmontra 7y agoTwo concurrent calls to that code. Both of them get the same balance (100), pass the test and deduct the amount (75), getting to a -50 balance.
- pmontra 7y agoI correct myself. It leads to a 25 balance but possibly a double spend of those 50 dollars in two transactions. The standard pattern is to use database row locking or calling a stored procedure that performs locking inside. Backend developers typically don't like the second solution but it provides a kind of API. Not to be overlooked if there are multiple services accessing it, especially in a polyglot environment.
- candu 7y agoDon't have separate get / set functions? async function changeBalance(account, amt) { /* * BEGIN; * * UPDATE accounts * SET balance = balance - amt * WHERE id = ${id}; * * COMMIT; */ } Of course, this is just a mutex in database transaction clothing :)
- winrid 7y agoUse the datastore... use atomic updates and transactions update balance, where balance >= amount, set balance = x
- random314 7y agoMutex specifically refers to OS/processor level constructs that protect critical sections using algorithms such as the bakery algorithm or special CPU instructions. None of these are required to protect the critical section in the example above. You can just use a busy flag. I am not familiar with JS, so this is approximate syntax. while (busy) { await busyIsFalse } busy = true Critical Section busy = false notify busyIsFalse Simple boolean flags will work