Empty rooms aren't deleted

[AnotherSymbiote]

I noticed a possible memory leak, so I tested by connecting to my server from about a dozen client webpages and repeatedly refreshing them. Memory usage increased with each refresh and never fell back to pre-refresh levels.

Turns out the unique rooms that are created for new connections aren't getting deleted — they remain in the room list despite becoming empty upon refresh. Empty rooms are supposed to be destroyed automatically, so I don't know what's going on, but it might be why memory usage keeps rising.

I tried explicitly deleting these rooms when a client disconnects. This removes them from the room list, but memory usage seems unaffected. I might be doing it wrong:

socket.on('disconnect', function() {
    delete io.sockets.adapter.rooms[socket.id];
});

Does this seem strange to anyone else? Is there another potential reason for the memory leak?

[dalesbit]

hm, its stuck in the garbage collection, u should try to assign null instead of deleting, try and share ur result ;)

[solosnet]

I also noticed io.sockets.adapter.rooms never cleans up automatically.

The above delete worked fine for me, though.

[AnotherSymbiote]

Turns out that what seemed like a leak was just normal Windows memory management! After more tests, I found that memory usage eventually plateaus.

I'll still explicitly clean up io.sockets.adapter.rooms, because it seems bad to have tons of empty rooms sitting around.

[ryanhestin]

I'd still like to know if this functionality is intentional and, if so, why it is this way. Does forcing explicit deletion lead to more possible use cases? That is to say: if they were deleted by default, having to re-add them back to accomplish some use case would be harder than the way it is now.

[rauchg]

Sockets should be automatically be cleaned up from rooms upon disconnection. If they stay, that's a bug.

[pmaoui]

Sockets aren't automatically cleaned up as of 1.0.6. I don't see that as a bug but it is less easy to deal with simple cases.
I have to check for now if my rooms are empty which is an hassle because io.adapter.rooms is an array of json meaning you have to count properties.

[gpestana]

Yep, this happen indeed. Not only when the rooms are automatically created when a user connects but also when rooms are explicitly created. In my case, if I do not delete the empty rooms explicitly, the memory usage increases quite a lot.
As @hestinr12 said, this can be seen as a functionality. But in my opinion, only when the rooms are created explicitly. Nevertheless, adding a removeAutomaticallyEmptyRoom flag or step to the API would be useful.

[ryanhestin]

I see this has been closed, but it does not appear to be resolved. Can we get some sort of comment on this please?

Will manually doing

socket.join(<room string>)

not auto delete a room upon disconnect?

Does this demand manual deletion? I'm seeing in my logs of a rather basic chat server that huge stacks of rooms are being kept in even after disconnection (force OR clean disconnect).

[rauchg]

My bad. Re-opening. rauchgbot closed based on the issue's age. Re-prioritizing for 1.3.0

[rase-]

I think this should be fixed by a recent-ish patch in socket.io-adapter, more specifically here.

Does the room not get cleaned up for you with the latest socket.io version, @hestinr12?

[barisusakli]

socket.io-redis doesn't use that version of socket.io-adapter specifically 0.3.1. socketio/socket.io-redis-adapter#42

[ryanhestin]

I'm currently rebuilding my dependencies and can report back tmrw with results

[rauchg]

We just published a new socket.io-redis

[AnotherSymbiote]

With Socket.IO 1.2.0, my empty rooms do get deleted automatically. I was glad to remove the workaround from my code. (Didn't do a regression test for 1.2.1, though.)

[rase-]

@SeijiSuenaga great to hear. We have automated tests in place to detect this since the fix.

Closing the issue for now. Let us know if it doesn't work for you @hestinr12!

[ryanhestin]

Tested out both 1.2.0 and 1.2.1, both do seem to clear the empty rooms upon disconnect.

However:

Whether this is intended or not I'm not entirely sure, but if you call

socket.on('disconnect', function(){ console.log(io['sockets']['adapter']['rooms']); })

the rooms that the socket which is disconnecting from still appear in the print out, yet their values will appear as empty arrays (indicating no remaining users). I see why this could be intended, but figured I'd mention it since we all already came this far >.>

[rauchg]

Are they still there if you nextTick the console.log ?

[rauchg]

I don't think they should be there as empty arrays either. If length == 0 we should delete the room?

[ryanhestin]

Yeah I'll have to retract that last comment, I rebuilt the whole project from the ground up and I don't see that happening anymore. Sorry 'bout that >.<

[rauchg]

<3

[ncacace]

My application uses randomly generated rooms. I use a variable to 'lease/reserve' rooms for a predetermined amount of time. Once the time is exceeded the room will be will be destroyed. The host is notified before this happens in case they want to extend the time.

Sometimes rooms require that data persistence be maintained if the room is destroyed. In this way if the room is regenerated at a later date the previous session's data can be repopulated. This persistent data needs to be deleted if the room is terminated.

I don't think it's right to automatically delete a room because it's not being used. I would add a switch (0,1,expiration datetimestamp), 0 being the default, when a room is created so that a pseudo garbage collector is notified whether a room should be set to null for collection by the Garbage collector.

ALSO... is the host creating a room where THEY dictate whether a room is to be destroyed once they no longer require it OR are they merely generating a room which will take on a life of its own; continuing to exist long after the host has left, as long as it is being used?

[darrachequesne]

@ncacace I think your use case is a bit too specific to be implemented in socket.io internals.

The room is destroyed once the last element leaves it (https://github.com/socketio/socket.io-adapter/blob/1.1.1/index.js#L81).