From 923a12e2de83ecaa75746a575e71a4739815d5c5 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Wed, 18 Sep 2024 15:56:20 +0200 Subject: [PATCH] fix(eio): discard all pending packets when the server is closed 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: https://github.com/socketio/socket.io/issues/5088 --- packages/engine.io/lib/socket.ts | 11 +++++- packages/engine.io/test/server.js | 61 +++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/packages/engine.io/lib/socket.ts b/packages/engine.io/lib/socket.ts index 3ec838e3ae..bfe6d3ba1d 100644 --- a/packages/engine.io/lib/socket.ts +++ b/packages/engine.io/lib/socket.ts @@ -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"; @@ -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); } @@ -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")); } diff --git a/packages/engine.io/test/server.js b/packages/engine.io/test/server.js index 25d87ce45e..35b1988357 100644 --- a/packages/engine.io/test/server.js +++ b/packages/engine.io/test/server.js @@ -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") {