lmammino / node-js-race-conditions

A series of examples illustrating race conditions in Node.js and potential solutions
MIT License
20 stars 5 forks source link

"Lets reintroduce deadlocks! Yay!" #1

Open zarutian opened 3 years ago

zarutian commented 3 years ago

Frankly I am not impressed by your article.

by replacing let balance = 0; with

const balance = (() => {
  let amount = 0;
  return Object.freeze({
    amount: () => amount,
    add: (addend) => { amount += addend; return amount; },
    subtract: (subtractend) => { amount -= subtractend; return amount; },
  });
})();

and do the needul code changes were balance is accessed, you completely bypass the need for mutexes.

Mutexes only make sense in low level code that is written in an programming languages lacking encapsulation like js has.

Sorry, but not sorry bad practices such as using mutex must be discauraged.

lmammino commented 3 years ago

That's a very valid point. Thanks for reading the article and for taking the time to create this issue.

The goal of balance in the example was just to simulate data available in a remote data source, therefore only accessible asynchronously.

One of the remarks we make at the end is to try to avoid implementing sequential access (in a mutex-like way) at all cost. We recommend preferring an "increment" primitive directly in the data source when that's available and we make an example with SQL.

I'd be happy to know what you think about this. I hope the article doesn't come through as a suggestion to use the mutex pattern everywhere without any due diligence on better alternatives...

zarutian commented 3 years ago

Something related worth a watch.

trasherdk commented 3 years ago

While reading the article, that I enjoyed, I was thinking the hole time why are database transactions not in here.

The mutex thing is a valid design choice for some middleware, but @zarutian's snippet gave me ideas to refactor some of my in memory accounting/auditing.