rabbitmq / rabbitmq-stream-dotnet-client

RabbitMQ client for the stream protocol
https://rabbitmq.github.io/rabbitmq-stream-dotnet-client/stable/htmlsingle/index.html
Other
122 stars 41 forks source link

Memory leak when connection to one of the brokers fails by producer\consumer #304

Closed MikhailTushev closed 1 year ago

MikhailTushev commented 1 year ago

Describe the bug

If there is a problem connecting to RabbitMQ in the LookupConnection and after 25 connection attempts with Address resolver, the connection does not close and throw exception, thereby causing a memory leak

Reproduction steps

  1. use address resolver (for example host first of node (not balancer or proxy))
  2. Leader need to be by 2 node
  3. After 25 retry we got to exception by RoutingClientException($"Could not find broker ({broker.Host}:{broker.Port}) after {maxAttempts} attempts")
  4. By second iteration of foreach brokers: connection success
  5. we see failed and successfully tcp connection in active connections

Expected behavior

we see one of successfully tcp connection in active connections

Additional context

class RoutingHelper, method LookupConnection

            while (broker.Host != advertisedHost || broker.Port != uint.Parse(advertisedPort))
            {
                ....
                if (attemptNo > maxAttempts)
                {
                    await client.Close("advertised_host or advertised_port doesn't match").ConfigureAwait(false); // here we close the failed connection that caused the memory leak

                    throw new RoutingClientException(
                        $"Could not find broker ({broker.Host}:{broker.Port}) after {maxAttempts} attempts");
                } 
                ....
            }
MikhailTushev commented 1 year ago

pull request for fix - https://github.com/rabbitmq/rabbitmq-stream-dotnet-client/pull/306

Gsantomaggio commented 1 year ago

closed via https://github.com/rabbitmq/rabbitmq-stream-dotnet-client/pull/306 thanks @MikhailTushev