CODEWITHSUNDEEP
javascriptasync-awaiteslintrace-condition
Run
Run

Reputation: 57196

Race condition error from ESLint require-atomic-updates when using object properties

The code below is causing a race condition when it is checked with ESLint, reporting a violation of the require-atomic-updates rule:

  let match = false

  try {
    match = await something()
  } catch (err) {
    // do something
  }
  if (match === false) {
    // do something
  }

What is the better way of writing this block of code?

Here's my actual code:

  let request = ctx.request.body || {}
  let password = request.password
  let match = false

  try {
    match = await bcrypt.compare(password, 'xxxxx')
  } catch (err) {
    ctx.throw(401, err)
  }
  if (match === false) {
    ctx.throw(401, 'invalid password')
  }

  ctx.body = {
    message: 'logged in ok'
  }

Error from ESLint:

Possible race condition: ctx.body might be reassigned based on an outdated value of ctx.body require-atomic-updates

Upvotes: 17

Views: 11534

Answers (3)

Jonas Wilms
Jonas Wilms

Reputation: 138277

You can safely ignore the warning :)

That ESLint is meant to catch things like this:

 let value = 0;

async function race() {
  value += await Promise.resolve(1);
  console.log(value);
}

race(); race();

In this case, race memoizes value on the stack, awaits a tick, then writes back to value. As other code runs in the meantime, value could've been changed, and then the update might be off ... it is not atomic.

In your case however, you read from ctx.request.body and write to ctx.body, so there is no non atomic update. Also, there is probably no other middleware acccessing the same ctx at the same time, so there cant be any concurrent modifications. So in your case it is a false positive, it is even questionable that this in any way positive (it might be a bug in ESLint).

Upvotes: 17

Sang
Sang

Reputation: 4456

I do not think this a bug. Let's assume your code snippet is included in an async function, namely doRequest.

As long as the ctx variable is defined outside of doRequest, the ctx.body assignment is in race condition. Because it is not possible to assure that the last value assigned to ctx.body belongs to the last call of doRequest.

I wrote a blog post about this race condition. There are 2 ways to avoid this warning

Method 1: move ctx into doRequest body, then return the ctx value at the end of the function.

Method 2: use promise-base-semaphore pattern

let ctxPromise

whenever you fire a request, do a call ctxPromise = doRequest().

Upvotes: 4

kyapwc
kyapwc

Reputation: 398

I realise it is a little bit late for this answer but just for any future users coming across this question, to disable this rule, in your .eslintrc.json or whichever relevant config you use, just specify:

"require-atomic-updates": "off"

Upvotes: 3

Related Questions