Skip to content

Commit

Permalink
fix(eio): discard all pending packets when the server is closed
Browse files Browse the repository at this point in the history
In some specific cases, the transport was not closed right away,
leaving the Node.js process alive even after closing the server.
The HTTP long-polling transport would be closed after the heartbeat
failure and the `closeTimeout` delay (20 + 25 + 30 seconds).

Example:

```js
io.on("connection", (socket) => {
  // the writeBuffer is not empty, so the transport is not closed right away
  io.close();
});
```

Related: #5088
  • Loading branch information
darrachequesne committed Sep 18, 2024
1 parent 13c6d2e commit 923a12e
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
11 changes: 9 additions & 2 deletions packages/engine.io/lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,13 @@ export class Socket extends EventEmitter {
* @return {Socket} for chaining
*/
public close(discard?: boolean) {
if (
discard &&
(this.readyState === "open" || this.readyState === "closing")
) {
return this.closeTransport(discard);
}

if ("open" !== this.readyState) return;

this.readyState = "closing";
Expand All @@ -570,7 +577,7 @@ export class Socket extends EventEmitter {
return;
}

debug("the buffer is empty, closing the transport right away", discard);
debug("the buffer is empty, closing the transport right away");
this.closeTransport(discard);
}

Expand All @@ -581,7 +588,7 @@ export class Socket extends EventEmitter {
* @private
*/
private closeTransport(discard: boolean) {
debug("closing the transport (discard? %s)", discard);
debug("closing the transport (discard? %s)", !!discard);
if (discard) this.transport.discard();
this.transport.close(this.onClose.bind(this, "forced close"));
}
Expand Down
61 changes: 61 additions & 0 deletions packages/engine.io/test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -1634,6 +1634,67 @@ describe("server", () => {
});
});

it("should discard the packets in the writeBuffer when stopping the server", (done) => {
engine = listen((port) => {
const clientSocket = new ClientSocket(`ws://localhost:${port}`);

clientSocket.on("data", () => {
done(new Error("should not happen"));
});

clientSocket.on("close", (reason) => {
expect(reason).to.eql("transport error");

clientSocket.close();
done();
});

engine.on("connection", (socket) => {
socket.write("hello");
engine.close();
});
});
});

it("should discard the packets in the writeBuffer when stopping the server (2)", (done) => {
engine = listen((port) => {
const clientSocket = new ClientSocket(`ws://localhost:${port}`);

clientSocket.on("data", () => {
done(new Error("should not happen"));
});

clientSocket.on("close", (reason) => {
expect(reason).to.eql("transport error");

clientSocket.close();
done();
});

engine.on("connection", (socket) => {
socket.write("hello");
socket.close(); // readyState is now "closing"
engine.close();
});
});
});

it("should not discard the packets in the writeBuffer when closing gracefully", (done) => {
engine = listen((port) => {
const clientSocket = new ClientSocket(`ws://localhost:${port}`);

clientSocket.on("data", (val) => {
expect(val).to.eql("hello");
done();
});

engine.on("connection", (socket) => {
socket.write("hello");
socket.close();
});
});
});

describe("graceful close", () => {
before(function () {
if (process.env.EIO_WS_ENGINE === "uws") {
Expand Down

0 comments on commit 923a12e

Please sign in to comment.