4 ms·

More subtly, here's an example of a hidden race condition in JS: async function totalSize(fol) { const files = await fol.getFiles(); let totalSize =

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

fourthark 7y ago
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!
- esprehn 7y ago
  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); }
  - evmar 7y ago
    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 https://es5.github.io/#x11.13.2
  - XCSme 7y ago
    What if you change it to totalSize = await file.getSize() + totalSize; Would it be fine then?
- deleted 7y ago
  [deleted]
tlrobinson 7y ago
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); }
brlewis 7y ago
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.

fourthark 7y ago

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!

esprehn 7y ago
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); }
- evmar 7y ago
  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 https://es5.github.io/#x11.13.2
- XCSme 7y ago
  What if you change it to totalSize = await file.getSize() + totalSize; Would it be fine then?
deleted 7y ago
[deleted]

tlrobinson 7y ago

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

brlewis 7y ago

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.