CODEWITHSUNDEEP
javascriptarraysnode.js
SLI
SLI

Reputation: 753

Array wont change it's data after splice. Scope problems?

I have a tcp server and i store all client data at clients array. So when some one disconnect i just splice them.

clients.splice(clients.indexOf(socket), 1);

I have a function that show all online users. It's just get data from that clients array and show it to me.

function showOnline(){
    console.log("Show user list");
    console.log("clients.length = " + clients.length);
}

clients array announced at the top of app.js file

var clients = []; <- here it is

var http = require('http');
var net = require('net');

So now when client disconnect i splice him and receive such log.

console.log("clients.indexOf(socket) = " + clients.indexOf(socket));
console.log("clients.length = " + clients.length);
clients.splice(clients.indexOf(socket), 1);
console.log("after splice clients.length = " + clients.length);

LOG
2016-03-04 22:06:33
clients.length = 2
after splice clients.length = 1

So it's only 1 user at clients array. And then i use my show online users function and it give me such log

LOG
2016-03-04 22:06:41 - Show user list
clients.length = 2

It's shows that there 2 users. How can that be? Is that scope problem? Or what can it be? Thank you for any reply.

Source. I think i do something wrong with sockets. This code work's but the longer it runs, the more I remain phantom clients at clients array.

var clients = [];
var packetReg = /(ART\D\d{4}(.*?)\DAND)/g;

var serverUrl = "http://localhost:4000/"

// Exit packet
var p = {
    exit: true
};
var ePacket = 'ART|' + randomNumber(1000, 9000) + JSON.stringify(p) + '|AND';

var fs = require('fs');
var http = require('http');
var https = require('https');
var net = require('net');
var url = require('url');

http.createServer(function (req, res) {
    var queryData = url.parse(req.url, true).query;
    res.writeHead(200, {"Content-Type": "text/html; charset=utf-8"});

    if (queryData.clean) {
        kick();
    } else if (queryData.expel) {
        kick();
    } else if (queryData.list) {
        showUserList();
    }
    else {
        res.writeHead(302, {'Location': 'https://localhost'});
        res.end();
    }

    function kick(all){
        if (queryData.expel){
            LOG("Expel user " + queryData.expel + " cmd");
        }
        else LOG("Kick all cmd");
        clients.forEach(function (client) {
            if (queryData.expel){
                if (client.key != queryData.expel) return;
            }
            client.write(ePacket);
        });
        if (queryData.expel){
            res.end("done");
        }
        else {
            cleanOnline();
            res.end("disconnected - " + clients.length);
        }
    }

    function showUserList(){
        LOG("Show user list");
        var temp = 'Clients: ' + clients.length + generateButton('?clean=1', 'Kick all') + '<br>';
        clients.forEach(function (client) {
            temp += 'User: ' + client.name + ' Key: ' + client.key + generateButton('?expel=' + client.key, 'Kick') + '<br>';
        });
        console.log("clients.length = " + clients.length);
        res.end(temp);
    }

    function generateButton(link, text){
        return ' <a href="' + serverUrl + link + '" target="_blank" onClick="window.location.reload()"><button>' + text + '</button></a> ';
    }
}).listen(4000, function(){
     console.log('listening http on port 4000');
});

var tcpSocket = net.createServer();
tcpSocket.on('connection', function (socket) {
    socket.setNoDelay(true);
    socket.banned = false;
    socket.isAdmin = false;
    socket.isModerator = false;
    socket.name = 'newUser'
    socket.key = ''
    socket.armbuf = '';

    clients.push(socket);

    LOG("New connection #" + clients.length);

    // Exit packet that close opened software
    socket.on('doExit', function () {
        LOG("Send exit packet");
        var p = {
            exit: true
        };
        socket.write('ART|' + randomNumber(1000, 9000) + JSON.stringify(p) + '|AND');
        socket.destroy();
    });
    // Init packet 
    socket.on('init', function (newData) {
        LOG("Init packet");

        //newData.data = packet with key from my program
        //I find out if users start my program few times then they will be added few times at clients array. 
        //So i want to prevent that with that function. It finds sockets with same key and just disconnect them except for the last socket.
        //
        FindAndCleanDuplicateSockets(newData.data, socket);

        var tempSocket = findSocket(socket);
        tempSocket.socket.key = newData.data;

        LOG("Send request to localhost about key " + tempSocket.socket.key);
        https.get('https://localhost/tgo/api/?apiRequest&key=' + tempSocket.socket.key, (res) => {
            var initData = '';
            res.on('data', function (chunk) {
                initData += chunk;
            });
            res.on('end', function() {
                LOG("Receive answer from localhost - " + initData.toString());
                var a = JSON.parse(initData.toString());
                if (a.data = "OK"){
                    tempSocket.socket.name = a.name;
                    tempSocket.socket.banned = !Boolean(a.chatBan);
                    if (a.type == "admin")
                        tempSocket.socket.isAdmin = true;
                    else if (a.type == "moderator")
                        tempSocket.socket.isModerator = true;

                    updateSocket(tempSocket);
                    broadcast(packetMaker(tempSocket.socket.name, 'start'), socket);
                }
                else socket.emit('doExit');
            });
        }).on('error', (e) => {
            socket.emit('doExit');
        });
    });

    //When user send ping packet
    socket.on('ping', function (newData) {
        //LOG("Ping packet");
        var p = {
            currentTime: currentTime()
        };
        socket.write('ART|' + randomNumber(1000, 9000) + JSON.stringify(p) + '|AND');
    });

    // When user change his name
    socket.on('newName', function (newData) {
        LOG("User change name from " + socket.name + " to " + newData.data);

        var tempSocket = findSocket(socket);
        tempSocket.socket.name = newData.data;
        updateSocket(tempSocket);
    });

    //When user send new message packet
    socket.on('newMsg', function (newData) {
        LOG("newMsg packet");
        var tempSocket = findSocket(socket);

        if (tempSocket.socket.banned) {
            LOG('User ' + tempSocket.socket.key + ' chat is banned');
            return;
        }

        var type = 'msg';
        if (tempSocket.socket.isAdmin)
            type = 'admin';
        else if (tempSocket.socket.isModerator)
            type = 'moderator';

        broadcast(packetMaker(tempSocket.socket.name, type, String(newData.data)), socket);
    });

    // Handle incoming messages from clients.
    socket.on('data', function (newData) {
        var d = String(newData);
        //LOG('Received data: ' + d);

        // I find that socket bacause i use buffer to store data. If i won't do that it will give me "is not defined" error
        var tempSocket = findSocket(socket);
        tempSocket.socket.armbuf += d;

        var dataArray = tempSocket.socket.armbuf.match(packetReg);
        if (dataArray != null && dataArray.length > 0){
            dataArray.forEach(function (match) {
                tempSocket.socket.armbuf = tempSocket.socket.armbuf.replace(match, "");
                if (match.startsWith('ART|') && match.endsWith('|AND')) {
                    var j = JSON.parse(cleanPacket(match));
                    switch (j.type) {
                        case 'init':
                          socket.emit('init', j);
                          break;
                        case 'ping':
                          socket.emit('ping', j);
                          break;
                        case 'newName':
                          socket.emit('newName', j);
                          break;
                        case 'newMsg':
                          socket.emit('newMsg', j);
                          break;
                        default:
                          break;
                    }
                }
                else console.log('Bad packet: ' + match);
                //LOG("armbuf.length = " + tempSocket.socket.armbuf.length);
            });
        }
        updateSocket(tempSocket);
    });

    socket.on('error',function(error) {
        socket.end();
    });

    socket.on('close',function() {
        var tempSocket = findSocket(socket);

        LOG("Send logout notification to localhost - " + tempSocket.socket.key);
        // Send info to api that user logout
        https.get('https://localhost/tgo/api/?logout&key=' + tempSocket.socket.key);
        // Broadcast data to all users that client is logout
        broadcast(packetMaker(tempSocket.socket.name, 'exit'), socket);

        console.log("clients.indexOf(socket) = " + clients.indexOf(socket));
        console.log("clients.length = " + clients.length);
        // Delete user from clients array
        clients.splice(clients.indexOf(socket), 1);
        console.log("after splice clients.length = " + clients.length);
        LOG("Close from API - " + tempSocket.socket.key);
        socket.destroy();
    });


    function cleanPacket(packet){
        packet = packet.replace("ART|", "");
        packet = packet.replace("|AND", "");
        return packet.substring(4);
    }
    function findSocket(socket){
        var socketData = {
            'id': clients.indexOf(socket),
            'socket': clients[clients.indexOf(socket)]
        };
        return socketData;
    }
    function FindAndCleanDuplicateSockets(key, exclude){
        clients.forEach(function (client) {
            if (client == exclude && client.key != key) return;

            LOG("User already exist in array. Delete old socket");
            client.emit('doExit');
        });
    }

    function findAllSocketsByKey(key, excludeSocket){
        var sockets = [];
        clients.forEach(function (client) {
            if (client == excludeSocket && client.key != key) return;
            sockets.push(client);
        });
        return sockets;
    }

    function updateSocket(tempSocket){
        clients[tempSocket.id] = tempSocket.socket;
    }

     // Send a message to all clients
    function broadcast(message, sender) {
        if (clients.length <= 0) return;

        LOG('broadcast ' + message);
        clients.forEach(function (client) {
            // Don't want to send it to sender
            if (client === sender) return;
            client.write(message);
        });
    }

    function packetMaker(userName, packetType, userMsg){
        var p = {
            currentTime: currentTime(),
            chat: {
                user: userName,
                type: packetType
            }
        };
        if (typeof userMsg != 'undefined')
            p['chat']['msg'] = userMsg;
        return 'ART|' + randomNumber(1000, 9000) + JSON.stringify(p) + '|AND';
    }
});


tcpSocket.listen(5000, function(){
     console.log('listening tcpSocket on port 5000');

     cleanOnline();
});

function cleanOnline(){
    console.log('Clean DB online');
    //https.get('https://localhost/tgo/api/?apiCleanOnline');
}

function LOG(data){
    console.log(currentTime() + " - " + data);
}

function randomNumber(min, max)
{
    return Math.floor(Math.random() * (max - min + 1)) + min;
}

function currentTime() {
    var date = new Date();

    var hour = date.getHours();
    hour = (hour < 10 ? "0" : "") + hour;

    var min  = date.getMinutes();
    min = (min < 10 ? "0" : "") + min;

    var sec  = date.getSeconds();
    sec = (sec < 10 ? "0" : "") + sec;

    var year = date.getFullYear();

    var month = date.getMonth() + 1;
    month = (month < 10 ? "0" : "") + month;

    var day  = date.getDate();
    day = (day < 10 ? "0" : "") + day;

    return year + "-" + month + "-" + day + " " + hour + ":" + min + ":" + sec;
}

Upvotes: 2

Views: 195

Answers (1)

Will
Will

Reputation: 1768

Here's your culprit:

function findSocket(socket){
    var socketData = {
        'id': clients.indexOf(socket),
        'socket': clients[clients.indexOf(socket)]
    };
    return socketData;
}

I see that you use this function to get the socket index in the array (referenced by property id) for later use.

Here's the problem.

Imagine this sequence of events in your app:

At this point you have 2 connections.

  1. Now receive new connection You just pushed a 3rd socket into the clients array tempSocket now has an id of 2

  2. The app calls an api to get a key

  3. An existing socket receives a disconnect event The app removes the existing socket from the clients array and you are down to 2 sockets. It has two because we have already added the new socket in bullet #1

  4. The api call in bullet #2 comes back with the key Now you call updateStocket(tempSocket) to update it. The problem is that tempSocket.id points to index 2, but you only have 2 elements in the array.

My sugguestion:

i. Change the clients array to object collection ii. When you get a new socket connection, use a library like UUID.js to get a unique hex and assign it to the new socket property iii. Add new socket to clients object using the hex id

If you decide to make the above change, you'll have to update your iteration method. I suggest use lodash to help you iterate your arrays and objects. It's really convenience. Otherwise, you'd have to get the object keys and iterate through them.

Good luck.

Upvotes: 1

Related Questions