Is my NodeJS promise code race condition safe?

Question

I am currently writing a small multiplayer game in nodejs.

When a player joins I want to load them onto the game world at their requested position. It is possible however that this position is occupied, if so I look around for the nearest available free space and load them at that location.

My way of doing this is as follows:

    function playerJoin(){    
        let playerTank = new Tank();
        this.findEmptyArea(playerTank, 100, 100).then((result) => {
            if (!result.success) {
                return;
            }
            addObjectToWorld(playerTank, result.x, result.y);    
        } 
    }


    function findEmptyArea(object, x, y){   
        return new Promise((resolve, reject) => {
          // Psuedo code: code iterates over game objects testing for 
          // collisions . if the requested position is free uses that else 
          // finds an empty nearby location and returns it
          return resolve({success: true, x: freeX, y: freeY});

          // fails to find empty location for object
          return resolve({ success: false });
        } 
  }

This is not the actual code but a stripped down version to make it clearer. My question is this:

When a user connects via the web socket, the function playerJoin runs. it then creates a new player tank , finds the free area and returns a promise, if it was successful the player tank is added to the world at the position.

Having looked closely at this I have wondered whether this code is flawed. Is it possible that addObjectToWorld is called on a location that is not actually free?

Such as the following:

1. player1 connects playerJoin called for player1
2. findEmptyArea called within playerJoin for player1
3. player 2 connects playerJoin called for player2
4. findEmptyArea called within playerJoin for player2
5. findEmptyArea promise for player1 finds a free space at 10,10 and returns promise.

6. findEmptyArea promise for player2 finds a free space at 10,10 and returns promise.

7. The .then() Promise code block in playerJoin (after findEmptyArea) for player 1 runs, putting the player at 10,10 via addObjectToWorld

8. The .then() Promise code block in playerJoin for player 2 runs, putting the player at 10,10 via addObjectToWorld.. and the game crashes

So I guess my question is, when a promise resolves, will the .then code block run immediately which runs addObjectToWorld runs straight away, or will other code potentially run first (such as another player also finding the area free)

Thank you for your help

Answer 1

Since findEmptyArea is async it most likely accesses something external, right?

Node.js is single threaded for executing JS code (even though there are experimental multithreading features in the latest versions). But Node.JS uses child threads to access external resources. So you can only have race conditions if you access external resources like files or do an API request.

If you do an API request the resource you access is in charge of ensuring that it returnes true only once to prevent race conditions.

But if you only check local objects within your Promises you shouldn't have any race conditions (as long as you don't play with setTimeout and stuff like that). The Node.JS event loop will execute the code for every resolved/rejected Promise one at a time. That means once a Promise is resolved the then code block is executed.

I found this article helpful about the event queue in combination with Promises.

The answer to your question depends on the way findEmptyArea checks for an empty area.

Some coding advice:

You don't need to use return when calling resolve and reject. And you might want to use reject since you mentioned the variable:

this.findEmptyArea(playerTank, 100, 100).then((result) => {
  addObjectToWorld(playerTank, result.x, result.y);    
}).catch((err) => {
  // findEmptyArea failed because the Promise was "rejected"
});

return new Promise((resolve, reject) => {
  // On success call this
  resolve({x: freeX, y: freeY});

  // On failure call this
  reject();
});

Answer 2

So I guess my question is, when a promise resolves, will the .then code block run immediately which runs addObjectToWorld runs straight away, or will other code potentially run first (such as another player also finding the area free)

Look this example:

'use strict';

async function work(i) {
  const interval = setInterval(() => { console.log('interval', i); }, 1);
  setImmediate(() => console.log('immediate', i));
  process.nextTick(() => console.log('nexttick', i));
  await findEmptyArea(i);
  addObjectToWorld(i, interval);
}

function addObjectToWorld(k, interval) {
  console.log('add object to the world', k);
  clearInterval(interval);
}

function findEmptyArea(k) {
  console.log('findEmptyArea resolving', k);
  return new Promise((resolve, reject) => {
    console.log('findEmptyArea executing', k);
    for (let i = 0; i < 1000000000; i++) {
      // high computation
    }
    resolve({ success: true, x: 1, y: 1 });
  });
}

for (let x = 0; x < 10; x++) {
  work(x);
}

The nextTick run before the the addObjectToWorld.

is it possible the nodejs event loop will allow something else to run first?

Looking at the example if you don't have async code (I/O) all will be sequential based on the event loop.

I put it inside the promise as playerJoin is going to be running often as new players connect and i don't want it blocking.

Consider that adding a Promise or async doesn't transform your code in non-blocking the event loop: you are still blocking the event loop also with the promises (check the for < 1000000000: it is blocking the event loop for all the connecting users.

In order to don't block the event loop you should run the high computation task in a worker_thread or child process.

If you do like this your event loop will not stop, so you can serve more users and scale up when needed.

But of course, in this case, you should implement a lock pattern like optimistic in order to book the free space.

Example:

'use strict';


const { Worker } = require('worker_threads');

async function work(i) {
  const interval = setInterval(() => { console.log('interval', i); }, 1);
  setImmediate(() => console.log('immediate', i));
  process.nextTick(() => console.log('nexttick', i));
  await findEmptyArea(i);
  addObjectToWorld(i, interval);
}

function addObjectToWorld(k, interval) {
  console.log('add object to the world', k);
  clearInterval(interval);
}

function findEmptyArea(k) {
  console.log('findEmptyArea resolving', k);
  return new Promise((resolve, reject) => {
    const worker = new Worker('./worker.js', { workerData: k });
    worker.on('message', resolve);
    worker.on('error', reject);
    worker.on('exit', (code) => {
      if (code !== 0) reject(new Error(`Worker stopped with exit code ${code}`));
    });
  });
}

for (let x = 0; x < 10; x++) {
  work(x);
}

// worker.js

'use strict';

const { workerData, parentPort } = require('worker_threads');

console.log('findEmptyArea executing', workerData);
for (let i = 0; i < 1000000000; i++) {
  // high computation
}
parentPort.postMessage({ hello: workerData });