connect-redis – how to protect the session object against race condition

I’m using nodejs with connect-redis to store the session data.

I’m saving user data in the session, and use it for in the session lifetime.

I’ve noticed that it’s possible to have a race condition between two requests that changes the session data.

I’ve tried to use redis-lock to lock the session, but it’s a bit problematic for me.

i don’t want to lock the entire session, but instead lock only specific session variable.

I found it to be impossible, and I thought about direction to solve it:

to stop using the session object to store user data, and save the variable directly in the redis and lock before using it.

I know that it can work, but it will require me to manage all the objects manually instead of just accessing redis through the session object.

Can you please share with me the best practice and your suggestions?

Thanks,
Lior

Here is Solutions:

We have many solutions to this problem, But we recommend you to use the first solution because it is tested & true solution that will 100% work for you.

Solution 1

Well, implementing your own storage might be the option for you. This documentation shows that all you need to do is to implement three methods: .get, .set and .destroy (see the last paragraph). It would be something like this (using node-redis library and modifying the original connect-redis store a bit):

var redis = require("redis"),
    redis_client = redis.createClient(),
    session_prefix = 'session::',
    lock_suffix = '::lock',
    threshold = 5000,
    wait_time = 250,
    oneDay = 86400;

/* If timeout is greater then threshold, then we assume that
   one of the Redis Clients is dead and he cannot realese
   the lock. */

function CustomSessionStore(opts) {
    opts = opts || {};
    var self = this;
    self.ttl = opts.ttl; // <---- used for setting timeout on session

    self.lock = function(sid, callback) {
        callback = callback || function(){};
        var key = session_prefix + sid + lock_suffix;
        // try setting the lock with current Date
        redis_client.setnx(key, Date.now( ), function(err, res) {
            // some error handling?
            if (res) {
                // Everything's fine, call callback.
                callback();
                return;
            }

            // setnx failed, look at timeout
            redis_client.get(key, function(err, res) {
                // some error handling?
                if (parseInt(res) + threshold > Date.now( )) {
                    // timeout, release the old lock and lock it
                    redis_client.getset(key, Date.now( ), function(err, date) {
                        if (parseInt(date) + threshold > Date.now()) {
                            // ups, some one else was faster in acquiring lock
                            setTimeout(function() {
                                self.lock(sid, callback);
                            }, wait_time);
                            return;
                        }
                        callback();
                    });
                    return;
                }
                // it is not time yet, wait and try again later
                setTimeout(function() {
                    self.lock(sid, callback);
                }, wait_time);
            });
        });
    };

    self.unlock = function(sid, callback) {
        callback = callback || function(){};
        var key = session_prefix + sid + lock_suffix;
        redis_client.del(key, function(err) {
            // some error handling?
            callback();
        });
    };

    self.get = function(sid, callback) {
        callback = callback || function(){};
        var key = session_prefix + sid;
        // lock the session
        self.lock(sid, function() {
            redis_client.get(key, function(err, data) {
                if (err) {
                    callback(err);
                    return;
                }
                try {
                    callback(null, JSON.parse(data));
                } catch(e) {
                    callback(e);
                }
            });
        });
    };

    self.set = function(sid, data, callback) {
        callback = callback || function(){};
        try {
            // ttl used for expiration of session
            var maxAge = sess.cookie.maxAge
              , ttl = self.ttl
              , sess = JSON.stringify(sess);

            ttl = ttl || ('number' == typeof maxAge
                  ? maxAge / 1000 | 0
                  : oneDay);

        } catch(e) {
            callback(e);
            return;
        }
        var key = session_prefix + sid;
        redis_client.setex(key, ttl, data, function(err) {
            // unlock the session
            self.unlock(sid, function(_err) {
                callback(err || _err);
            });
        });
    };

    self.destroy = function(sid, callback) {
        var key = session_prefix + sid;
        redis_client.del(key, function(err) {
            redis_client.unlock(sid, function(_err) {
                callback(err || _err);
            });
        });
    };
}

Side note: I didn’t implement error handling for .lock and .unlock. I’m leaving this up to you! 🙂 There might be some minor mistakes (I don’t have NodeJS at the moment and I’m writing this from my memory 😀 ), but you should understand the idea. Here’s the link which contains the discussion about how to use setnx for locking/unlocking Redis.

The other note: you would probably want to make some custom error handling for routes, because if any route throws an exception, then Redis session won’t be unlocked. The .set method is always called as the last thing in route – opposite to .get method which Express calls at the very begining of the route (that’s why I lock at .get and unlock at .set). Still you will be locked only for 5 seconds, so it does not have to be a problem though. Remember to tune it to your needs (especially threshold and wait_time variables).

Final note: with this mechanism your request handlers will only fire one after another per user. This means, that you will not be able to run concurrent handlers per user. This might be a problem, so the other idea is to held the data outside the session and handle locking/unlocking manually. After all, there are some things which have to be handled manually.

I hope it helps! Good luck!

Note: Use and implement solution 1 because this method fully tested our system.
Thank you 🙂

All methods was sourced from stackoverflow.com or stackexchange.com, is licensed under cc by-sa 2.5, cc by-sa 3.0 and cc by-sa 4.0

Leave a Reply