Fix race condition in leaving rooms

[sharat87]

Hi team!

This is a quick small PR to address an error that we are seeing more and more often recently. The error looks like this:

TypeError: Cannot read property 'size' of undefined
    at Adapter._del (/app/node_modules/socket.io-adapter/dist/index.js:67:37)
    at Adapter.delAll (/app/node_modules/socket.io-adapter/dist/index.js:83:18)
    at Socket.leaveAll (/app/node_modules/socket.io/dist/socket.js:190:22)
    at Socket._onclose (/app/node_modules/socket.io/dist/socket.js:334:14)
    at Client.onclose (/app/node_modules/socket.io/dist/client.js:245:20)
    at Socket.emit (events.js:412:35)
    at Socket.onClose (/app/node_modules/engine.io/lib/socket.js:348:12)
    at Object.onceWrapper (events.js:519:28)
    at WebSocket.emit (events.js:400:28)
    at WebSocket.onClose (/app/node_modules/engine.io/lib/transport.js:106:10)

It looks like we are checking the room exists in the rooms Map, and if it does, we proceed to do a few things. But when multiple sockets are removed from a room independently, then this is causing a race condition on this line:

https://github.com/socketio/socket.io-adapter/blob/master/lib/index.ts#L91

This PR addresses this by,

1. Calling .get on the Map exactly once.
2. Emitting the delete-room even if the deletion actually happened.

However, I'm a bit skeptical of this as well. While I believe this is the underlying reason for this error, I don't hard evidence to prove it, other than staring at the code really hard and coming up with a scenario where the error happens.

So, if you've already seen this error in the past, any advice would be much appreciated. I tried searching the repository for this error message but couldn't find anything relevant.

Thank you very much for your time, and of course, for your work on socket.io! 🚀

[darrachequesne]

Hi! Thanks for the pull request 👍

Merged as 912e13a, though I'm not sure how it could happen (the object being defined here and then undefined here 👀 )

[sharat87]

Hey, thanks for merging! Super extra thanks for also pushing a release 🎉

Yeah bugs due to race conditions have that peculiarity. They don't seem possible just from looking at the code. But we have situations where several multiple socket connections leave rooms at the same time, and this keeps showing up. We can't reproduce it predictably though.

I think the underlying reason is about multiple asynchronously invoked function calls, trying to update and read the same Map object.