'Leak' of channels (IModel instances) in case where connection is blocked

[neilgreatorex]

Hi,

I think I've come across an issue whereby channels can be 'leaked' during a connection that has been blocked by the broker due to resource constraints. The issue occurs if you try to create new channels while the connection is blocked. The calls to CreateModel() throw a System.TimeoutException and you (quite reasonably) don't get a reference to the channel.

The problem is that once the connection is unblocked, suddenly the channels that you tried to create appear on the broker end and are not cleaned up by any garbage collection in the client. Looking at the heap at this point, the problem seems to be that the sessions created in the SessionManager are not cleaned up. As far as I can see, there's no way for a user of the client library to access the SessionManager via public API.

I've added my sample code to this gist. In the gist is also the results that I get on the console when running the code against a resource constrained (blocked) broker (along with the rabbitmqctl commands that were run and their output).

Please let me know if you want me to raise an issue here on GitHub for this, or if I should post this elsewhere.

Thanks,
Neil

PS. Merry Christmas to those who celebrate 😃🎅

[michaelklishin]

How do you suggest it should be addressed?

[neilgreatorex]

My suggestion would be that the code in the CreateModel() method (https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/client/impl/Connection.cs#L205) should look something like:

public IModel CreateModel()
{
    EnsureIsOpen();
    ISession session = CreateSession();
    try
    {
        var model = (ModelBase)Protocol.CreateModel(_factory, session);
        model.ContinuationTimeout = _factory.ContinuationTimeout;
        model._Private_ChannelOpen();
        return model;
    }
    catch (Exception)
    {
        session.Close(...);
        throw;
    }
}

I haven't tested this and I'm not sure if there would be any issues with it, but that was my first idea.

Cheers,
Neil