Race condition in JavaScript with compound assignment

Question

I'm not talking about complex race conditions involving the network or events. Rather, I seem to have found out that the += operator is not atomic in V8 (Chrome 58, or Node 8).

The code below aims to run two so-called threads in parallel. Each "thread" calls repeatedly a function that returns its number parameter after sleeping that many seconds. The results are summed up into an accumulator.

function sleep(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

// Return the passed number after sleeping that many seconds
async function n(c) {
  await sleep(c * 1000);
  console.log('End', c);
  return c;
}

let acc = 0;  // global

// Call n repeatedly and sum up results
async function nForever(c) {
  while (1) {
    console.log('Calling', c);
    acc += await n(c);  // += not atomic?!
    console.log('Acc', acc);
  }
}

(async function() {
  // parallel repeated calls
  nForever(1);
  nForever(5.3);  // .3 for sanity, to avoid overlap with 1 * 5
})();

The problem is that after ~5 seconds, I'd expect the accumulator to be 10.3 (5 times 1 + 1 times 5.3). However, it's 5.3!

I'm curious how the implementation is behind the scenes, maybe a closure is created during an await statement that stores all variables that are currently in scope at the time of the await call. It is quite interresting to see that both functions seem to increase the acc completely independently, like they put acc in their local scope instead of the global one — Icepickle, Jun 02 '17 at 22:12

score 7 · Accepted Answer · edited Jun 02 '17 at 22:48

7

This is not a race condition, because you are explicitly yielding the execution using await.

The standard defines that a compound assignment such as += is not atomic: The left-hand-side of a compound assignment is evaluated before the right-hand-side.^[1]

So if your RHS changes acc somehow, the changes will be overwritten. Most simple example:

var n = 1;
n += (function () {
    n = 2;
    return 0;
})();

console.log(n);

edited Jun 02 '17 at 22:48

Dan Dascalescu

143,271
52
317
404

answered Jun 02 '17 at 22:25

Kijewski

25,517
12
101
143

1

Thanks for pointing to the standard. It shows that my [assembly language driven mental model of the compound assignment](https://stackoverflow.com/questions/44337989/race-condition-in-javascript-with-compound-assignment/44338120#44338120) was incorrect (broken by step 5, to be precise). – Dan Dascalescu Jun 04 '17 at 21:33

score 3 · Answer 2 · answered Jun 02 '17 at 22:10

3

Indeed, after replacing the acc += await n(c) line with:

const ret = await n(c); acc += ret;

the race condition was avoided.

My guess is V8 didn't optimize acc += await n(c) to an ADD of the n() result over the memory location containing acc, but rather expanded it to acc = acc + await n(c);, and the initial value of acc when nForever(5.3) was first called, was 0.

This is counter-intuitive to me, though not sure the V8 developers would consider it a bug.

answered Jun 02 '17 at 22:10

Dan Dascalescu

143,271
52
317
404

1

interestingly, if I transform your original code through babeljs website, it is actually working as you would expect it, giving 10.3 after 5.3 seconds – Icepickle Jun 02 '17 at 22:28

Race condition in JavaScript with compound assignment

2 Answers2

Linked