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

[Bug]: HttpClients not disposed #1647

Open
Timovzl opened this issue Jan 30, 2024 · 0 comments
Open

[Bug]: HttpClients not disposed #1647

Timovzl opened this issue Jan 30, 2024 · 0 comments
Labels

Comments

@Timovzl
Copy link

Timovzl commented Jan 30, 2024

Describe the bug 🐞

It appears that the source-generated clients receive ownership of an injected HttpClient, which they do not dispose. In fact, these clients do not implement IDisposable at all. HttpClient is IDisposable, however.

Should HttpClient always be disposed? No, not if it originated from the DI container, which is then responsible for its lifetime. In that case, it must not be disposed. But in the scenario where the code just created a new HttpClient and gave it to the source-generated client to own, then that client becomes responsible for the disposal.

Especially with the generated clients currently being registered with transient lifetime, a lot of undisposed HttpClient instances can be accumulated.

Step to reproduce

  • Call services.AddRefitClient<IWhatever>().
  • Use Go To Implementation (usually Ctrl+F12) to view the source-generated implementation.
  • Observe how the type is not IDisposable, and yet it injects a disposable resource, the HttpClient.
  • See also how the generated client is registered with transient lifetime and is fed a brand new HttpClient that is otherwise abandoned.

Reproduction repository

https://github.com/reactiveui/refit

Expected behavior

Preferably, an HttpClient not owned by the container should not be injected (see #1646).

Alternatively, an injected HttpClient not owned by anything else should become owned by the generated client, requiring it to implement IDisposable to dispose of the resource. This would solve the problem when parent services or the generated client have scoped lifetime, but not for transient parents! A transient parent that is IDisposable needs to be disposed by the caller. Not only is this often overlooked, but it is also hard to do if the interface being implemented itself does not implement IDisposable. This shows why the former solution is far, far preferable.

As such, solving #1646 appropriately may make the current issue obsolete. However, the issue seemed significant and complex enough to warrant a separate ticket.

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

Refit Version

No response

Additional information ℹ️

No response

@Timovzl Timovzl added the bug label Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant