moscajs / aedes

Barebone MQTT broker that can run on any stream server, the node way
MIT License
1.75k stars 228 forks source link

[bug] duplicated closed clients #928

Closed Gianluca-Casagrande-Stiga closed 4 months ago

Gianluca-Casagrande-Stiga commented 6 months ago

System Information

Describe the bug The Aedes.clients object can have many closed clients that are not removed from the list after disconnection. The number of closed clients increase with usage, potentially causing memory leak. It's only a broker side issue: on the client side connection is ok. The closed client list can be cleared by broker restart, or manually deleting the elements of broker.clients where closed = true.

Unfortunately I haven't been able to reproduce the problem yet on dev env. Studying my production logs, I've found that the closed client creation in the list is related to the error "connection closed" on write.js.

I think the issue is related to many disconnection/reconnection of the same client in a cluster env. These connections could be managed by different brokers at the same time. The result is, for example, that the same client ID is closed in broker1.clients and it is connected in broker2.clients.

Expected behavior A closed client shouldn't remain in the broker client list.

Workaround When the closed client list is too long, I execute a function to clear the list.

Question Does anyone have the same problem? I'll keep working on this and I will write here if there will be any updates.

robertsLando commented 6 months ago

@Gianluca-Casagrande-Stiga The only place where client is removed is:

https://github.com/moscajs/aedes/blob/981d071e189309797c3227c9f105862cc23c2576/lib/client.js#L326-L328

That calls:

https://github.com/moscajs/aedes/blob/981d071e189309797c3227c9f105862cc23c2576/aedes.js#L310-L318

I'm not sure if in the first if the that._authorized check could cause some memory leaks, could you try to see if removing it creates any issue and if it fixes your problem?