[HN Gopher] Node.js race conditions
___________________________________________________________________
Node.js race conditions
Author : todsacerdoti
Score : 70 points
Date : 2021-01-25 14:16 UTC (8 hours ago)
(HTM) web link (www.nodejsdesignpatterns.com)
(TXT) w3m dump (www.nodejsdesignpatterns.com)
| applepple wrote:
| Callbacks (including those passed to Promise then methods) are a
| dangerous primitive. I would recommend using awaitable streams
| like those used in https://socketcluster.io/ - Or
| https://github.com/SocketCluster/writable-consumable-stream for
| general purpose.
|
| The philosophy of SC and WritableConsumableStream is that the
| main trunk logic of your system/module should declared as a
| single serial stream and any parallel logic should explicitly
| branch off from it.
|
| With traditional event listeners, there is no such thing as a
| 'main trunk' of the logic... The entry points (event listeners)
| are scattered all over the place and this can cause a lot of
| possible state permutations when business logic is executed
| inside those listeners (since you can't control the timing of
| when those event handlers are called).
|
| This kind of programming is a good alternative to functional
| programming as it offers a safe way to asynchronously mutate
| state.
| hyeomans wrote:
| Nice, are you using it in production? First time I hear about
| SocketCluster
| jkrems wrote:
| Nice reminder that different kinds of race conditions exist, even
| without shared mutable data!
|
| One thing I wish the article mentioned: In the given example,
| another solution would have been to make the balance change safe.
| E.g. `addBalance()` or `updateBalanceIfEqual(old, new)`.
| Requiring a mutex around each call site that needs to be used
| correctly (and keeps being used correctly over time) seems like a
| pretty fragile way out.
| moocowtruck wrote:
| i feel like this post creates the problem, and then solves the
| wrong problem
| xsmasher wrote:
| Yep - in order for this solution to work, all of the calls need
| access to the same mutex; they all need to be in the same
| process (this is noted toward the end).
|
| If you can do that, just share an "account" object. Calling
| account.balance+=50 doesn't require a mutex.
| SparkyMcUnicorn wrote:
| Chromium is rendering the code blocks as single-line, and they're
| impossible to read.
|
| Is it just me? Firefox seems to load them just fine.
| city41 wrote:
| They look fine for me in Chromium.
| loige wrote:
| One of the authors here. Could you please provide more detailss
| (system, browser type and version) and maybe a screenshot?
|
| Do you have JS disabled?
| chrisseaton wrote:
| > A race condition is a type of programming error
|
| I don't think a race condition is inherently an error. If you can
| design your program to accommodate races, including even data
| races, then allowing them and not spending time on protecting
| against them can be a useful performance optimisation.
|
| It's only a bug if the race causes you to get into a state that
| you don't want.
| dimitrios1 wrote:
| I seriously doubt people writing web software are planning for
| intentional races in their system designs the way an OS dev
| does to avoid synchronization overhead.
| kube-system wrote:
| I don't await promises if I don't _need_ to. Don 't think
| that's a super unusual pattern.
| rileymat2 wrote:
| I feel a bit ignorant, although you don't need to await on
| assignment, don't you need to await to "unwrap" it when
| needed?
| thecopy wrote:
| Sometimes you dont need the result.
| rileymat2 wrote:
| Right, if you don't need it, you don't need to unwrap it.
| IggleSniggle wrote:
| As the article is making a round-about way of pointing
| out, you _might_ need to "block" (unwrap) due to a side-
| effect in order to make the sequencing correct for
| downstream logic that touches the same thing.
|
| Shouldn't be necessary if things are coded appropriately
| in the first place, and maybe better to just refactor,
| but...
| hexsprite wrote:
| It's just a function that returns a promise object. The
| await keyword triggers the wait and unwrap functionality,
| but in cases where you don't care you can just call it as
| a regular function
| aaomidi wrote:
| You should however, catch it.
|
| If you're doing nothing with a promise (awaiting, or
| catching), then you have bad code.
|
| You can read more about that here:
| https://palantir.github.io/tslint/rules/no-floating-
| promises...
| piterrro wrote:
| Mutex in Nodejs, ROTFL. This kind of bad programming leads to
| overuse of synchronization primitives. Nodejs is single threaded
| and doesnt need locks and mutexes. In any application. Properly
| writing code will make synchronization unnecessary. In the
| provided example, the saveBalance function should not overwrite
| the values of balance variable but rather increment its value
| (increment with negative too). That's it your problem is solved,
| one line of code to change.
| lostcolony wrote:
| That assumes that the underlying implementation of saveBalance
| can be updated to push the atomic operation downstream.
|
| In general you highlight the problem though; without the
| central datastore ensuring atomicity, this isn't really solving
| anything.
|
| I.e., if this app had multiple instances, you STILL have a
| problem, even with this solution, since nothing guarantees that
| another -instance- hasn't also loaded the old data.
| piterrro wrote:
| Exactly, the author at the end explains that his approach
| doesnt work when you're using cluster/pm2/worker because
| these are separate processess, that just makes me laugh again
| as the whole post is pointless.
| dboreham wrote:
| The (somewhat contrived) example is trying to model what
| happens when the state change is done with I/O (hence by
| definition asynchronous). Your idea to make the state changes
| idempotent is a good one, but not typically possible.
| piterrro wrote:
| Read the post title once again: "Node.js race conditions",
| why mention I/O when the post is not about it?
| aaomidi wrote:
| Race conditions are in the nature of concurrent code. Not just
| parallel code.
|
| Take go, use gomaxprocs and set it to 1. Spawn a bunch of
| goroutines that share memory. And easily you'll have race
| conditions if you're not careful.
| piterrro wrote:
| I didnt say race conditions are impossible in Nodejs. Just
| highlighted that good program engineering is enough to avoid
| it and programmers should do that in the first place.
| aaomidi wrote:
| Sure but at the same time there is a reason synchronization
| primitives exist.
|
| Readability of code > everything else.
| piterrro wrote:
| Nodejs doesn't have synchronization primitives, just
| because you use Promise and assign it to variable "mutex"
| doesn't make it any more real
| emteycz wrote:
| What do you think is the real thing? It's also a value in
| a memory cell.
| aaomidi wrote:
| Yup :)
|
| https://github.com/golang/go/blob/master/src/sync/mutex.g
| o
|
| You can easily see what Go does here. It's not magic.
| [deleted]
| dboreham wrote:
| It would be better if I had the time today to refute this
| in detail, because it's important that others reading this
| thread know this assertion is false, but...it's false.
| cphoover wrote:
| Agreed Over the past 10 years, I've done async code to
| coordinate lots of different concurrent tasks in JS, never
| once needed a the use of a mutex.
| hrishi wrote:
| You're right that this example is better solved without
| mutexes, but it's good that the article is introducing the
| concept to NodeJS devs who may not be used to it. Heck, I used
| mutexes extensively for years in C code and even I didn't know
| they were an option.
|
| That said, mutexes are also not always wrong. Compare-and-swap
| is good, ensuring atomicity is good, but sometimes a mutex is
| what you need, especially if you're fixing legacy code with
| concurrency issues.
| golergka wrote:
| To have a race condition, you first have to have a shared state.
| And in my experience, for almost all applications where NodeJS is
| a good fit (usually a CPU-light application that is serving web
| requests, each of them independent of others, and waiting for a
| long time on IO, usually different database requests and other
| external services), shared state is already a bad pattern.
| gpderetta wrote:
| shared state and message passing are dual to each other.
| chrisseaton wrote:
| > To have a race condition, you first have to have a shared
| state.
|
| I think you're possibly confusing 'race condition' with 'data
| race'. You need shared state for a data race, but not for a
| race condition.
|
| For example if you send a request to two web services, then
| wait for responses from either, then that's a race condition on
| which result comes back first, because the non-deterministic
| timing can change program behaviour. And that's not even
| necessarily a problem or a bug.
|
| You can easily get race conditions even in a system that tries
| to minimise shared state, such as Erlang, even though they
| protect you from data races.
| dnautics wrote:
| Technically in your example the shared state is the network.
| chrisseaton wrote:
| No the situation still holds if you send two requests on
| different network cards to entirely different networks that
| share nothing.
| dnautics wrote:
| In that case the shared state is the observer.
| chrisseaton wrote:
| I don't think that's really a meaningful or useful
| definition of shared state for these purposes.
| mfbx9da4 wrote:
| Cool article! Fun programming assignment - implement a Mutex
| using promises.
| paxys wrote:
| In a real world use case you are going to have multiple instances
| of your application running at the same time anyways, so the
| mutex solution is pointless. Use a DB transaction or something.
| tored wrote:
| DB transaction will not save you in this case.
|
| As the article hinted you have to either update the balance in
| one query or use a lock, like an optimistic lock.
|
| Optimistic lock could have also been used instead of the mutex.
| lostcolony wrote:
| If you have multiple instances, a lock of any sort won't
| work; you have to update the query.
|
| If you update the query, a lock is unnecessary.
|
| I think it's fair to assume the parent was inferring that if
| you were using a DB transaction, you'd rewrite the logic to
| use it, rather than keeping the read and the write separate
| from each other, and therefore make a transaction pointless.
| tored wrote:
| No, locks work. Lock is needed if you need to do some sort
| of complex computation before writing, like updating a JSON
| blob.
| lostcolony wrote:
| As both I and the parent said, "If you have multiple
| instances".
|
| In which case, locks (like those mentioned in the
| article) don't work. Because instance A reads 0, instance
| B reads 0, instance A writes 50, instance B writes 50.
| Totally serialized on each instance, totally
| unpredictable in when they hit the DB.
|
| Just like you have a lock serving as a synchronization
| point on an instance, in the case of multiple instances,
| you want to push the synchronization point down into a
| single instance at the data layer (which you often have
| when using a traditional RDBMS). But RDBMS' don't have
| locks, per se, for that synchronization; they have
| transactions that serve a similar purpose.
|
| But, regardless of where that synchronization is, you
| have to ensure the update logic stays there. Having an
| interface that loads data, and saves data, at the
| application layer, ensures that a DB layer lock won't
| work, either. You have to change it to have the DB
| actually read and update the data to benefit from the
| synchronization the DB transaction provides you.
|
| So, as I said before...a lock (at the application layer)
| is neither sufficient nor necessary. A DB transaction
| (with the implied update of the query necessary to make a
| transaction mean anything) is both.
| tored wrote:
| Article talks about two different types locks, one is the
| mutex, which the article admits won't work when using
| with multiple instances.
|
| The other is briefly mentioned with a link to Wikipedia
| and is a optimistic lock that is to be used with your
| database.
|
| I talked about two use cases for optimistic locking, one
| applied with a database, that with will work with any
| number of instances, because the synchronization check
| lives in the database and not in the instance.
|
| My other case was to use an optimistic lock instead of
| the mutex example, that will still only work with single
| instance because the synchronization check is local to
| the instance. (Edit: wrong, it could actually work there
| too, if saveBalance() was changed)
|
| Optimistic locking is quite simple to implement, just
| check that the value has not changed before writing, if
| it has you have to reread the value again.
| tored wrote:
| Because you have updated your reply with more information
| I make a new reply instead. My old reply still stands
| about the what the article said and my use of optimistic
| locking.
|
| Databases do support locks, usually called pessimistic
| locking (opposite of optimistic locking). In a RDBMS that
| is usually a table lock or a row lock.
|
| Table locks are usually bad for performance and row locks
| better, however both are clumsy to work with, this is
| where the simplicity of optimistic locking comes in.
|
| Where pessimistic locking is part of the RDBMS itself,
| optimistic locking is something that you implement
| yourself on top of the RDBMS.
|
| Optimistic locks are lightweight (optimal case one read
| and one write) and easy to implement and understand.
|
| In the example in the article I would have argued for
| optimistic locking on all cases and it would have worked
| with multiple instances.
|
| Transactions and locks does not serve a similar purpose.
| cryptica wrote:
| It's important to figure out ahead of time which features of your
| system can be executed in parallel and which features should be
| executed serially.
| divs1210 wrote:
| Mutexes in a single threaded event loop? Wut?
|
| This problem requires a compare-and-swap primitive, not mutexes.
___________________________________________________________________
(page generated 2021-01-25 23:02 UTC)