HttpResponseMessage object is not disposed and missing optional CancellationToken for asynchronous calls.

[HoPe0211]

First problem statement.

In the RestService.cs file, the HttpResponseMessage instances created within the RequestAsync<T> and RequestListOfAsync<T> methods are not properly disposed of. This can lead to several issues:

1. Resource Leaks: HttpResponseMessage holds unmanaged resources such as network connections and memory buffers. Failing to dispose of these instances can result in resource leaks, which can degrade application performance over time.
2. Memory Consumption: Accumulation of undisposed HttpResponseMessage objects can lead to increased memory usage, potentially causing the application to run out of memory, especially under high load or long-running operations.
3. Connection Exhaustion: HttpClient uses a connection pool to manage network connections. Not disposing of HttpResponseMessage can exhaust the available connections in the pool, leading to timeouts and failed requests.
4. Unpredictable Behavior: Undisposed resources can cause unpredictable behavior in the application, making it difficult to diagnose and fix issues. To address these issues, it is essential to ensure that HttpResponseMessage instances are properly disposed of after use. This can be achieved by using the using statement or explicitly calling the Dispose method on the HttpResponseMessage objects.

Second problem statement. Lack of usage CancellationToken in RestService.cs and CachedSchemaRegistryClient.cs async operations.

The CancellationToken allows for the graceful cancellation of asynchronous operations. This is particularly useful in scenarios where a request needs to be aborted due to user action, timeout, or other conditions.

Both issues are addressed in the attached patch file. The code modifications are backward compatible and provide a default value, if the client does not set it.

0001-Add-CancellationToken-support-to-async-methods.patch

[AndreReise]

+1 Having cancelable async operations is great - I wish the Kafka client had more methods like that. However, extending a method signature with a CancellationToken might break binary compatibility, right?

[rayokota]

Thanks @HoPe0211 , I've fixed the possible resource leak here: https://github.com/confluentinc/confluent-kafka-dotnet/pull/2447

We'll consider the other changes in the next major version bump, thanks

[HoPe0211]

+1 Having cancelable async operations is great - I wish the Kafka client had more methods like that. However, extending a method signature with a CancellationToken might break binary compatibility, right?

I think, if you are using the built-in classes, the binary compatibility should not be broken because these are optional parameters. The binary compatibility might breakes if you have a custom implementation from ISchemaRegistryClient and you are using that. In this case, you need to extend your implementation with optional CancellationToken parameter. However, this is a simple and quick change.

[HoPe0211]

Thanks @HoPe0211 , I've fixed the possible resource leak here: #2447

We'll consider the other changes in the next major version bump, thanks

Thanks for considering my suggestions.

I have one more suggestion, which depends on this patch file what I have already shared in this issue. Do you want me to create another issue on top of this?

I would share one more patch file, and it would be awesome, if you considered that change as well in the next major build.

[rayokota]

@HoPe0211 , sure, I can look at the other suggestion in another patch. I guess I missed it.

[HoPe0211]

@HoPe0211 , sure, I can look at the other suggestion in another patch. I guess I missed it.

Hi @rayokota ,

0001-Add-CancellationToken-support-to-async-serializer-me.patch

The above attached patch file was created on top of my previous patch file. It would be awesome if these 2 patch files were considered in the next major bump. Together these patch files enable the clients to correctly utilize the async operations and pass an optional CancellationToken for long-running operations.

Thanks and best regards!