HttpConnection leaks UnobservedTaskExceptions

[MihaZupan]

If the SocketsHttpHandler is not disposed, HttpConnection.Dispose will not be called in time to observe the exception from the readAheadTask. The Dispose will run as part of the finalizer, but since the TaskExceptionHolder in _readAheadTask is also finalizable, it may be too late as the execution order of finalizers is not guaranteed.

Impact: Noise
The exceptions that leak to UnobservedTaskExceptions would be ignored anyway, so there is no adverse side effects aside from noise.

I am able to reproduce the same thing on 5.0 as well. The code is pretty much the same on 3.1, so I assume it would repro there as well (haven't tried since I am relying on the ConnectCallback).

Example stack trace:

System.AggregateException: A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host..)
 ---> System.IO.IOException: Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host..
 ---> System.Net.Sockets.SocketException (10054): An existing connection was forcibly closed by the remote host.
   --- End of inner exception stack trace ---
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource<System.Int32>.GetResult(Int16 token)
   at System.Net.Security.SslStream.ReadAsyncInternal[TIOAdapter](TIOAdapter adapter, Memory`1 buffer)
   at System.Net.Http.HttpConnection.<CheckUsabilityOnScavenge>g__ReadAheadWithZeroByteReadAsync|44_0()
   --- End of inner exception stack trace ---

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

If the SocketsHttpHandler is not disposed, HttpConnection.Dispose will not be called in time to observe the exception from the readAheadTask. The Dispose will run as part of the finalizer, but since the ExceptionHolder in _readAheadTask is also finalizable, it may be too late as the execution order is not guaranteed.

Impact: Noise
The exceptions that leak to UnobservedTaskExceptions would be ignored anyway, so there is no adverse side effects aside from noise.

I am able to reproduce the same thing on 5.0 as well. The code is pretty much the same on 3.1, so I assume it would repro there as well (haven't tried since I am relying on the ConnectCallback).

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[karelz]

Triage: Might be fixed by #60729
No large impact, Future.

[davidfowl]

Why not backport this one? It'll end up in people's logs (they're logging unobserved tasks).

[MihaZupan]

I think we should fix this in 8.0. Leaking internal exceptions like this is ugly.

[ramonsmits]

I observe a lot of these via FirstChanceException logging:

System.IO.IOException: Unable to read data from the transport connection: Operation canceled.
 ---> System.Net.Sockets.SocketException (125): Operation canceled
   --- End of inner exception stack trace ---
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource<System.Int32>.GetResult(Int16 token)
   at System.Net.Http.HttpConnection.<CheckUsabilityOnScavenge>g__ReadAheadWithZeroByteReadAsync|44_0()

I discovered that these are related to pooling. I can significantly reduce these by setting the PooledConnection* properties:

var handler = new SocketsHttpHandler
{
    PooledConnectionIdleTimeout = TimeSpan.FromHours(1),
    PooledConnectionLifetime = TimeSpan.FromHours(1),
};

Issue is that I cannot control all created HttpClients.