4

This is my server side websocket script:

var clients = [ ];

//sample request: ****:8080/?steamid=123456789
var connection;
var aqsteamid = getParameterByName("steamid",request.resource);

connection = request.accept(null, request.origin); 

connection.ID = aqsteamid;
connection.balRefreshes = 0;
connection.clientIndex = clients.push(connection) - 1;

//check if this user is already connected. If yes, kicks the previous client ***====EDITED====***
for(var i = 0; i < clients.length; i++)
{
    if(clients[i].ID === aqsteamid){
        var indx = clients.indexOf(clients[i]);
        clients[indx].close();
    }
}

console.log('ID',connection.ID,' connected.');


socket.on('close', function(webSocketConnection, closeReason, description){
    try{
        console.log('ID',webSocketConnection.ID,'disconnected. ('+closeReason+';'+description+')');
        webSocketConnection.balRefreshes = 0;
        webSocketConnection.spamcheck = false;
        clients.splice(webSocketConnection.clientIndex, 1);
    }catch(e)
    {
        console.log(e);
    }
});

Basically what I want is to kick all connections with same ID (for example, connecting with multiple browser tabs).

But, instead of kicking the old client, it kicks both clients or in some cases both clients remain connected with same ID.

Is there any other way or is there any mistakes in my script?

Thanks

Ned
  • 361
  • 3
  • 16
  • 2
    `clients.splice(webSocketConnection.clientIndex, 1);` is the problem. `splice()` changes the index of elements in front of it, so your `clientIndex` property doesn't line up with `clients` for very long, which explains the erratic symptoms described. you probably need to use `clients.indexOf()` to find the connection's current index before you `splice()` – dandavis Mar 12 '16 at 15:27
  • 2
    as an aside, it would be better to use an object with ID keys instead of looping through an array every connection event. `used={};.. if(used[aqsteamid]){...}else{used[aqsteamid]=connection}` and on disconenect, it's simpler to `delete` than `splice`: `delete used[webSocketConnection.ID];` – dandavis Mar 12 '16 at 15:30
  • 1
    hey, thanks for your reply. I've changed the loop a little bit but it's still the same. Does it have to be done with your **used=[]** example? – Ned Mar 12 '16 at 16:08
  • 1
    Only one thing you can do, first check the connection is exist of not then connect, so there no chance to create another client with same id – Mukul Medatwal Mar 12 '16 at 17:20
  • 1
    @dandavis I forgot to change the script in on('close') event, seems to be working fine now. Thanks, please write an answer so I can accept it. – Ned Mar 12 '16 at 17:43
  • @dandavis has the correct answer, it would be a shame for someone else to answer and get the credit, though it is tempting :) – Scott Mar 12 '16 at 17:46
  • @dandavis - please post your comments as an answer, both for future readers and for putting the bounty to rest. – Myst Mar 12 '16 at 19:49

2 Answers2

1

using an object instad of Array to key the clients pool makes it faster and simpler:

var clients = {};

//sample request: ****:8080/?steamid=123456789
var connection;
var aqsteamid = getParameterByName("steamid",request.resource);

connection = request.accept(null, request.origin); 

connection.ID = aqsteamid;
connection.balRefreshes = 0;
clients[aqsteamid]=connection;

socket.on('close', function(webSocketConnection, closeReason, description){
    try{
        console.log('ID',webSocketConnection.ID,'disconnected. ('+closeReason+';'+description+')');
        webSocketConnection.balRefreshes = 0;
        webSocketConnection.spamcheck = false;
        delete clients[aqsteamid];
    }catch(e)
    {
        console.log(e);
    }
});

//check if this user is already connected. If yes, kicks the previous client 
if(clients[aqsteamid]) clients[aqsteamid].close();
console.log('ID',connection.ID,' connected.');

With an object pool, we can remove all the array pool looping and comparing logic, and our index will never get out of sync.

dandavis
  • 16,370
  • 5
  • 40
  • 36
0

It sounds like multiple connections with the same ID could be part of a genuine workflow with multiple tabs (unlike, say, malicious users that intentionally scrape data w/multiple threads...)

Rather than "kicking" the users from other tabs and then having to deal with them re-connecting, a more elegant solution would be to introduce an orchestration layer across multiple tabs.

You can rely on localstorage api to elect a master tab that will handle communications with the server (doesn't really matter if it's websocket or ajax) and share responses with other tabs - again, through localstorage. It doesn't really matter if you have 1 or 20 tabs open when you can share that data since you care about same message notifications, or stock ticker updates, or whatever.

From another stackoverflow answer:

The storage event lets you propagate data between tabs while keeping a single SignalR connection open (thereby preventing connection saturation). Calling localStorage.setItem('sharedKey', sharedData) will raise the storage event in all other tabs (not the caller):

$(window).bind('storage', function (e) {
    var sharedData = localStorage.getItem('sharedKey');
    if (sharedData !== null)
        console.log(
            'A tab called localStorage.setItem("sharedData",'+sharedData+')'
        );
});

Given the code above, if sharedKey value is already available when the page is loaded, assume a master tab is active and get shared values from localstorage. You can check if a master tab re-election is needed (i.e. that browser tab has been closed or navigated away) with an interval or relying on something more sophisticated like page visibility api.

Note you're not limited to sharing "same" data across multiple tabs but instead batch any requests over a shared channel.

Community
  • 1
  • 1
Oleg
  • 24,465
  • 8
  • 61
  • 91