You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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. HttpClientisIDisposable, 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.
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
The text was updated successfully, but these errors were encountered:
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 implementIDisposable
at all.HttpClient
isIDisposable
, 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 newHttpClient
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
services.AddRefitClient<IWhatever>()
.IDisposable
, and yet it injects a disposable resource, theHttpClient
.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 implementIDisposable
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 isIDisposable
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 implementIDisposable
. 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
The text was updated successfully, but these errors were encountered: