DarkWanderer / ClickHouse.Client

.NET client for ClickHouse
MIT License
302 stars 58 forks source link

Request hangs until timeout if the previous request has thrown an exception #485

Closed Inaceat closed 1 week ago

Inaceat commented 3 weeks ago

I make two consecutive requests to a database:

using var httpHandler = new HttpClientHandler() { MaxConnectionsPerServer = 1, };// limited to 1 for simplicity.
using var httpClient = new HttpClient(httpHandler) { Timeout = TimeSpan.FromSeconds(7) };

using var connection = new ClickHouseConnection(connectionString, httpClient);

try
{
    using var command1 = new ClickHouseCommand(connection) { CommandText = request1 };
    using var reader1 = command1.ExecuteReader();
}
catch (Exception e) { ... }

try
{
    using var command2 = new ClickHouseCommand(connection) { CommandText = request2 };
    using var reader2 = command2.ExecuteReader();
}
catch (Exception e) { ... }

If the first one is made to a table with columns of a type that is not supported by the ClickHouse.Client, the second one is hanging until HttpClient's Timeout is reached.

The issue seems to be caused by the fact that ClickHouseDataReader's constructor, which is called inside command1.ExecuteReader(), may throw an exception without proper cleanup. It receives HttpResponseMessage and is responsible for its disposal. If creation of ClickHouseDataReader throws, reference to reader1 is lost, and its HttpResponseMessage cannot be disposed. The connection used by the first request remains open: изображение MaxConnectionsPerServer is reached, and the second request cannot start, causing "Timeout reached" exception.

This issue reproduces only when targeting .NetFramework.

Full sample project: Sample.zip

DarkWanderer commented 2 weeks ago

I have released 7.5.1 with a speculative fix - although I have to admit, I didn't try your example fully due to lack of time. Is the situation better with 7.5.1?

Inaceat commented 1 week ago

New version resolves the issue, thank you.