Skip to content

Commit

Permalink
fix: keep track of active sockets
Browse files Browse the repository at this point in the history
When a given socket was disconnected, either by the server-side or by the client-side, the manager was closed too, regardless of the other connected sockets.

```js
const socket1 = io({
  autoConnect: false
});
const socket2 = io("/test");

socket1.disconnect(); // also disconnect socket2
```

This bug was introduced in [1].

[1]: b60e909
  • Loading branch information
darrachequesne committed Dec 7, 2020
1 parent ec1f8c3 commit f8f60fc
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 17 deletions.
27 changes: 10 additions & 17 deletions lib/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ export class Manager extends Emitter {
private _reconnectionDelayMax: number;
private _timeout: any;

private connecting: Array<Socket> = [];
private encoder: Encoder;
private decoder: Decoder;
private skipReconnect: boolean;
Expand Down Expand Up @@ -601,19 +600,6 @@ export class Manager extends Emitter {
if (!socket) {
socket = new Socket(this, nsp, opts);
this.nsps[nsp] = socket;
var self = this;
socket.on("connecting", onConnecting);

if (this._autoConnect) {
// manually call here since connecting event is fired before listening
onConnecting();
}
}

function onConnecting() {
if (!~self.connecting.indexOf(socket)) {
self.connecting.push(socket);
}
}

return socket;
Expand All @@ -626,9 +612,16 @@ export class Manager extends Emitter {
* @private
*/
_destroy(socket: Socket) {
const index = this.connecting.indexOf(socket);
if (~index) this.connecting.splice(index, 1);
if (this.connecting.length) return;
const nsps = Object.keys(this.nsps);

for (const nsp of nsps) {
const socket = this.nsps[nsp];

if (socket.active) {
debug("socket %s is still active, skipping close", nsp);
return;
}
}

this._close();
}
Expand Down
8 changes: 8 additions & 0 deletions lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ export class Socket extends Emitter {
];
}

/**
* Whether the Socket will try to reconnect when its Manager connects or reconnects
*/
public get active(): boolean {
return !!this.subs;
}

/**
* "Opens" the socket.
*
Expand Down Expand Up @@ -343,6 +350,7 @@ export class Socket extends Emitter {
* @private
*/
private onconnect(id: string) {
debug("socket connected with id %s", id);
this.id = id;
this.connected = true;
this.disconnected = false;
Expand Down
27 changes: 27 additions & 0 deletions test/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,33 @@ describe("connection", function () {
});
});

it("should not close the connection when disconnecting a single socket", (done) => {
const manager = new Manager({
autoConnect: false,
});
const socket1 = manager.socket("/foo");
const socket2 = manager.socket("/asd");

socket1.connect();
socket1.on("connect", () => {
socket2.connect();
});

socket2.on("connect", () => {
socket2.on("disconnect", () => {
done(new Error("should not happen for now"));
});
socket1.disconnect();
setTimeout(() => {
socket2.off("disconnect");
manager.on("close", () => {
done();
});
socket2.disconnect();
}, 200);
});
});

// Ignore incorrect connection test for old IE due to no support for
// `script.onerror` (see: http://requirejs.org/docs/api.html#ieloadfail)
if (!global.document || hasCORS) {
Expand Down

0 comments on commit f8f60fc

Please sign in to comment.