Autopruning of Empty Rooms No Longer Works

[ssiano]

I believe this was introduced by commit 7793a33, which should be reverted.

!arr.length works for an array as the author expected, but !obj.length for an object always evaluates as true as shown by the snippet below.

this.rooms[room], i.e., io.sockets.adapter.rooms[room], is an object (see commit ce93e2a).

Thus a room was being autopruned the first time someone in that room disconnected. The next client to disconnect triggered an attempt to delete a room that had already been deleted, thereby crashing the app.

The error message was TypeError: Cannot convert null to object.

The error occurred on Line 85 of index.js.

// JavaScript Snippet

// Arrays
var arr = [ "a", "b", "c" ];
var arrResult = ( !arr.length ) ? true : false;
console.log( "Array: " + arrResult ); // Array: false

// Objects
var obj = {
a: "one",
b: "two",
c: "three"
};
var objResult = ( !obj.length ) ? true : false;
console.log( "Object: " + objResult ); // Object: true

[ssiano]

Further testing revealed that we should use

if (!Object.keys(this.rooms[room]).length)

on Lines 66 and 88 instead of

if (!this.rooms[room].length)

So reverting commit b49b781 is not the right approach.

// JavaScript Snippet

// Arrays
var arr = [ "a", "b", "c" ];
console.log( arr.length ); // 3
console.log( !arr.length ); // false

var emptyArr = [];
console.log( emptyArr.length ); // 0
console.log( !emptyArr.length ); // true

// Objects
var obj = {
a: "one",
b: "two",
c: "three"
};
console.log( obj.length ); // undefined
console.log( !obj.length ); // true
console.log( Object.keys(obj).length ); // 3
console.log( !Object.keys(obj).length ); // false

var emptyObj = {};
console.log( emptyObj.length ); // undefined
console.log( !emptyObj.length ); // true
console.log( Object.keys(emptyObj).length ); // 0
console.log( !Object.keys(emptyObj).length ); // true

[ssiano]

I have only been working with JavaScript for a few months and Socket.IO for a few days, so I don't feel comfortable submitting a pull request. However, I believe there are two more issues with delAll(). Below is a proposed change to address all three issues, as shown by the comments.

I continue to get crashes when a user disconnects, but these same to be resolved by the fixes below.

I hope this helps.

Adapter.prototype.delAll = function(id, fn){
  var rooms = this.sids[id];
  if (rooms) {
    for (var room in rooms) {
      if (this.rooms.hasOwnProperty(room)) { // changed rooms to this.rooms
        delete this.rooms[room][id];
        // Moved the autopruning of the room inside the
        // this.rooms.hasOwnProperty(room) check
        if (!Object.keys(this.rooms[room]).length) { // uses Object.keys to get an array
            delete this.rooms[room];
        }
      }
    }
  }
  delete this.sids[id];
};

[rase-]

Thanks a ton for reporting @ssiano! This should be addressed in #12.