StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.9k stars 1.51k forks source link

AbortOnConnectFail=false causes memory leak when server accepts tcp connection but doesn't reply to redis commands #1458

Closed maksimkim closed 1 year ago

maksimkim commented 4 years ago

Repro client: https://gist.github.com/maksimkim/a9df89dd083ab8477a9ccebf25cecc83 server: tcp server that accepts connection but doesn't reply to any protocol message, e.g. dotnetty discard client (change port and disable tls in config): https://github.com/Azure/DotNetty/blob/dev/examples/Discard.Server/Program.cs

PhysicalBridge indefinitely transitions between Connecting->ConnectedEstablishing->Disconnected but PhysicalConnection objects are not getting disposed.

maksimkim commented 4 years ago

@mgravell , @NickCraver , any chance somebody can take a look?

Gevil commented 4 years ago

Looks like this might actually be happening to us as well.

We started seeing memory usage climbing up since we started using stackexchange redis recently.

image

We are also using abortConnect = false. Connection multiplexer is forcibly reconnected already due to intermittent connection issues.

There are a lot of issues to be honest using this at the moment :\ Trying to wrap around this multiplexer in all kinds of ways to keep it stable somehow.

Gevil commented 4 years ago

Our Redis Package version is: 2.1.58

maksimkim commented 4 years ago

@Gevil , yeah, we ended up creating wrapper around multiplexer that enforces abortOnConnect=true, subscribes for ConnectionFailed event and recreates multiplexer on failure.

maksimkim commented 3 years ago

@mgravell , @NickCraver gentle ping

NickCraver commented 2 years ago

Missed this after closing - there was a reference chain issue in older versions that's now resolved and this should no longer happen in new versions. You still shouldn't see disconnect/reconnects like this so often (unless you're putting an unreasonably low timeout vs. your latency and/or load), but when it does happen, the leak should no longer be an issue :)

NickCraver commented 1 year ago

We found this had a new cause here, addressed in #2338.