0

I have a Node.js server running the following code, which uses socket.io library :

var Common = require('./common');
var _ = require('lodash');
var store = (function() {
    var userMapByUserId = {};
    var tokenSet = {};
    var generateToken = function(userId) {
        if (_.has(userMapByUserId, userId)) {
            var token = '';
            do {
                token = Common.randomGenerator();
            } while (_.has(tokenSet, token));
            if (userMapByUserId[userId].tokens === undefined) {
                userMapByUserId[userId].tokens = {};
            }
            userMapByUserId[userId].tokens[token] = true;
        }
    };
    var deleteToken = function(userId, tokenValue) {
        if (_.has(userMapByUserId, userId)) {
            if (userMapByUserId[userId].tokens !== undefined) {
                userMapByUserId[userId].tokens = _.omit(userMapByUserId[userId].tokens, tokenValue);
            }
            if (_.has(tokenSet, tokenValue)) {
                tokenSet = _.omit(tokenSet, tokenValue);
            }
        }
    };
    return {
        generateToken: generateToken,
        deleteToken: deleteToken
    };
}());

module.exports = function(socket, io) {
    socket.on('generateToken', function(ownerUser) {
        store.generateToken(ownerUser.userId);
        io.sockets.emit('userList', {
            userMapByUserId: store.getUserList()
        });
    });
    socket.on('deleteToken', function(token) {
        store.deleteToken(token.userId, token.tokenValue);
        io.sockets.emit('userList', {
            userMapByUserId: store.getUserList()
        });
    });
};

So, basically we can have multiple clients sending requests to this server to add/remove tokens. Do I need to worry about synchronization and race conditions ?

Arian
  • 7,397
  • 21
  • 89
  • 177

1 Answers1

2

I don't see any concurrency issues in the code you've shown.

The Javascript in node.js is single threaded and works via an event queue. One thread of execution runs until it completes and then the JS engine fetches the next event waiting to run and runs that thread of execution until it completes. As such, there is no pre-emptive concurrency in Javascript like you might have to worry about in a language or situation using threads.

There are still some places in node.js where you can get opportunities for concurrency issues. This can happen if you are using asynchronous operations in your code (like async IO to a socket or disk). In that case, your thread of execution runs to completion, but your async operation is still running and has not finished yet and has not called its callback yet. At this point, some other event can get processed and other code can run. So, if your async callback refers to state that could get changed by other event handlers, then you could have concurrency issues.

In the two socket event handlers you disclosed in your question, I don't see any asynchronous callbacks within the code running in the event handlers. If that's the case, then there are no opportunities for concurrency there. Even if there were concurrency opportunities, but the separate pieces of code weren't using/modifying the same underlying data, then you would still be OK.


To give you an example of something that can cause a concurrency issue, I had a node.js app that was running on a Raspberry Pi. Every 10 seconds it would read a couple of digital temperature sensors and would record the data in an in-memory data structure. Every few hours, it would write the data out to disk. The process of writing the data out to disk was all done with a serios of async disk IO operations. Each async disk IO operation has a callback and the code continued only within that callback (technically I was using promises to manage the callbacks, but its the same concept). This create a concurrency issue because if the timer that records new temperatures fires while I'm in the middle of one of these async disk operations, then the data structure could get changed in the middle of writing it to disk which could cause me problems.

It wasn't that the async operation would get interrupted in the middle of it's disk write (node.js is not pre-emptive in that way), but because the whole operation of writing to disk consisted of many separate async file writes, other events could get processed between the separate async file writes. If those events could muss with your data, then it could create a problem.

My solution was to set a flag when I started writing the data to disk (then clear the flag when done) and if I wanted to add something to the data structure while that flag was set, then the new data would go into a queue and would be processed later so the core data structure was not changed while I was writing it to disk. I was able to implement this entirely in a few methods that were used for modifying the data so that the calling code didn't even need to know anything different was going on.


Though this answer was written for Ajax in a browser, the event queue concept is the same in node.js so you may find it helpful. Note that node.js is called "event IO for V8 Javascript" for a reason (because it runs via an event queue).

Community
  • 1
  • 1
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • So , if I work with 10 different data structures (maps, arrays, etc) but I don't have any such async piece of code , everything is ok , right ? and by the way, doesn't this make the code very slow ? let's assume that we have 10,000 users all working at the same time. – Arian Jul 28 '15 at 00:55
  • 1
    @ArianHosseinzadeh - single threaded does not necessarily mean "slow". It can be a lot more efficient than running lots of concurrent threads in terms of total throughput. It does mean that requests have to get in line to be processed. But each individual request gets processed pretty fast. Whether this works or not depends entirely upon how fast the requests come in and how long they take to process. When you exceed the capacity of a single thread, it is common to implement nodejs clustering (multiple server processes working on requests). – jfriend00 Jul 28 '15 at 00:57