Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disposable interfaces #964

Merged
merged 4 commits into from
Nov 23, 2020

Conversation

trejjam
Copy link
Contributor

@trejjam trejjam commented Aug 17, 2020

What kind of change does this PR introduce?
Feature, resolve #660. A generated client has an implementation of IDisposable.Dispose (if interface is marked as IDisposable)

What is the current behavior?
IDispose is not generated, when necessary.

What is the new behavior?
Interface marked with IDisposable has implemented IDispose method

void IDisposable.Dispose()
{
    Client?.Dispose();
}

What might this PR break?
Nothing

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:
This feature is handling following messages when AddRefitClient is used for registration (HttpClient is created using IHttpClientFactory)

[15:18:02 DBG] Microsoft.Extensions.Http.DefaultHttpClientFactory
        HttpMessageHandler expired after 120000ms for client '...'
[15:18:12 DBG] Microsoft.Extensions.Http.DefaultHttpClientFactory
        Starting HttpMessageHandler cleanup cycle with 2 items
[15:18:12 DBG] Microsoft.Extensions.Http.DefaultHttpClientFactory
        Ending HttpMessageHandler cleanup cycle after 0.6748ms - processed: 0 items - remaining: 2 items

@bennor
Copy link
Contributor

bennor commented Aug 17, 2020

Weirdly, HttpClient is not supposed to be disposed. From the official docs:

HttpClient is intended to be instantiated once and re-used throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors.

@trejjam
Copy link
Contributor Author

trejjam commented Aug 18, 2020

Hi, yes the default (empty) constructor should not. HttpClient created through HttpClientFactory has shorter lifetime with different internal behaviour.

@iansteed
Copy link

iansteed commented Aug 25, 2020

@trejjam is correct, a generated client is injected with a new HttpClient instance each time it's created. The instance contains client specific base address, timeout, headers, and pipeline handlers. The underlying HttpMessageHandler is performing the actual requests and is managed by the HttpClientFactory.

https://docs.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests

Learn how to use IHttpClientFactory, available since .NET Core 2.1, for creating `HttpClient` instances, making it easy for you to use it in your applications.

@bennor
Copy link
Contributor

bennor commented Aug 25, 2020

Looks like I have some reading to do. 😳

@clairernovotny clairernovotny changed the base branch from master to main November 13, 2020 13:50
@clairernovotny clairernovotny merged commit e17aced into reactiveui:main Nov 23, 2020
@trejjam trejjam deleted the feature/disposable-interfaces branch November 23, 2020 18:38
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IDisposable Interfaces
4 participants