Fix bug caused by EventEmitter placement

[Soulike]

I think the reason for the bug described in #74 is that the EventEmitter calls all listeners synchronously.

The EventEmitter calls all listeners synchronously in the order in which they were registered.

So at runtime, the testcase

adapter.on("leave-room", (room, sid) =>
{
    adapter.del("s1", "r1");
});

adapter.addAll("s1", new Set(["r1"]));
adapter.del("s1", "r1");

makes the code of _del() equivalents to

_del(room, id) {
    if (this.rooms.has(room)) {
        const deleted = this.rooms.get(room).delete(id);
        if (deleted) {
            ((room, sid) => 
            {
                adapter.del("s1", "r1");  // 1. does this.rooms.delete(room), and returns;
            })()  
        }
        // 2. After the listener execution, this.rooms.get(room) === undefined
        if (this.rooms.get(room).size === 0) {
            this.rooms.delete(room);
            this.emit("delete-room", room);
        }
    }
}

Therefore I think the bug actually does not involves any asynchronously invoked function calls. The listeners of leave-room are always executed synchronously before the following codes in _del().

If I modify the testcase to

adapter.on("leave-room", (room, sid) =>
{
    // makes the call asynchronous
    setImmediate(() => {
        adapter.del("s1", "r1");
    });
});

adapter.addAll("s1", new Set(["r1"]));
adapter.del("s1", "r1");

the error will never happen again.

---

What I am concerning is that event listeners can modify the resources used by following codes and makes them buggy.

For the fixed version of _del()

_del(room: Room, id: SocketId) {
    const _room = this.rooms.get(room);
    if (_room != null) {
        const deleted = _room.delete(id);
        if (deleted) {
            // Listeners here can modify the resources used by following codes
            this.emit("leave-room", room, id);
        }
        if (_room.size === 0 && this.rooms.delete(room)) {
            this.emit("delete-room", room);
        }
    }
}

I suggest that the event listeners should be emitted after all resource-related jobs are done, like

_del(room: Room, id: SocketId) {
    const _room = this.rooms.get(room);
    if (_room != null) {
        // all resource-related jobs are guaranteed not to be disturbed
        const deletedSocketIdFromRoom = _room.delete(id);
        let deletedRoom = false;
        if (_room.size === 0 && this.rooms.delete(room)) {
            roomDeleted = true;
        }

        if (deletedSocketIdFromRoom) {
            this.emit("leave-room", room, id);
        }
        if (deletedRoom) {
            this.emit("delete-room", room);
        }
    }
}

The same principle also applies to the error caused by the following test case I found

const adapter = new Adapter({server: {encoder: null}});

adapter.on('create-room', () =>
{
    adapter.del('s1', 'r1');
});

adapter.addAll('s1', new Set(['r1']));

/*
            if (!this.rooms.get(room).has(id)) {
                                     ^

TypeError: Cannot read properties of undefined (reading 'has')
*/

I edited the code and made this pr to solve the problem. Thank you!

[darrachequesne]

I think this can be closed now. Please reopen if needed.