Hacker News Reader

Comment by millstone | Hacker News Reader

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?

View on HN

I 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.

More 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;
  }

Because 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!

The 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);
  }

Wow, why would you define it this way. I had to check the spec because I didn't believe you. They got #2/#3 backwards.

https://es5.github.io/#x11.13.2

What if you change it to

  totalSize = await file.getSize() + totalSize;

Would it be fine then?

I 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);
  }

This 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.

I 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.

Yep, 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...

If 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.

Don'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 :)

>> 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.

How 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...
        }
    }

This is getting off topic, but what would probably be best here is a test and set operation: setBalance(expectedBalance, newBalance)

How 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");
      })
    }

And 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.

Operational 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?

SQL.

>it comes with a number of performance and memory/storage consumption implications.

Instead of selecting sum(), CREATE TRIGGER then and update a singleton on insert. You can even create a view with N recent transactions and catch inserts into it to maintain that N, if you don’t want a full history.

Mutex 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

Use the datastore... use atomic updates and transactions update balance, where balance >= amount, set balance = x

Can anyone explain this to me, how is it a race condition?

Two 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.

I 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.

Reply on news.ycombinator.com