Socket gateway using ws accumulates connections (possible memory leak?)

jblyberg commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

When using ws as a socket server, disconnected sessions seem to accumulate and the listeners are not flushed after client disconnects, resulting in MaxListenersExceededWarning errors:

(node:66205) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connection listeners added to [WebSocketServer]. Use emitter.setMaxListeners() to increase limit

Minimum reproduction code

https://stackblitz.com/edit/nestjs-typescript-starter-zu55y9

Steps to reproduce

Run the reproduction code
Use postman (or whatever) to repeatedly connect and disconnect to the socket server
Observe console output. It should be something like this:

GW: Client disconnect
GW: Client connect
GW: Client disconnect
GW: Client connect
GW: Client connect
GW: Client disconnect
GW: Client connect
GW: Client connect
GW: Client connect
GW: Client disconnect
GW: Client connect
GW: Client connect
GW: Client connect
GW: Client connect
... etc ...
(node:66205) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connection listeners added to [WebSocketServer]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
... etc ...

Expected behavior

Client connect and disconnect should be the same
No errors

Package

[ ] I don't know. Or some 3rd-party package
[ ] code>@nestjs/common</code
[ ] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[X] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[X] code>@nestjs/websockets</code
[ ] Other (see below)

Other package

No response

NestJS version

9.2.1

Packages versions

    "@nestjs/common": "^9.2.1",
    "@nestjs/core": "^9.2.1",
    "@nestjs/platform-express": "^9.2.1",
    "@nestjs/platform-ws": "^9.2.1",
    "@nestjs/websockets": "^9.2.1",

Node.js version

19.3.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

SirReiva commented 1 year ago

handleConnection method already caught socket connection, you dont need attach listener again in each socket connection and this listener you nerver remove manually

kamilmysliwiec commented 1 year ago

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

nestjs / nest