Repository navigation
[QUIC] Finalize QUIC dispose model #32142
Description
Socket currently has a behavior of throwing SocketError.OperationAborted for outstanding operations when Dispose() is called.
We need to decide how Dispose() will trigger outstanding operations:
QuicListener-- outstandingAcceptConnection()calls.QuicConnection-- outstandingConnect(),AcceptStream(),Close()calls. Plus any already open streams.QuicStream-- outstandingRead(),Write(),Shutdown(),Abort()calls.
The current model throws an OperationAbortedException, mimicing the Socket API; We should look at how consistent we want to be here, and if we want to adjust for usability/perf reasons, such as having AcceptConnection() or having AcceptStream() return null rather than throwing.
We should also consider how this interacts with a user-initiated connection/stream abort, and with a server-initiated connection/stream abort. In sockets, a variety of error codes can be thrown for these, one of them being SocketError.OperationAborted.
Activity
[-]Finalize QUIC dispose model[/-][+][QUIC] Finalize QUIC dispose model[/+]GSPP commentedon Jun 12, 2020
What would this do if
Disposeis called right before an operation is started? Would this throwObjectDisposedException? If yes, this would cause an inherent race condition. For example, a caller ofSocket.Acceptcannot know whether he will receiveSocketError.OperationAbortedor an ODE. That makes theSocketError.OperationAbortednot useful for handling this condition.This is a thorny issue for
Socketand it seems to apply just as much to QUIC. There has been some extensive discussion which might be useful to take note of: https://github.com/dotnet/corefx/issues/5749Having an orderly was for shutting down is quite important as essentially any application must shut down eventually. This issue affects many users of
Socketand QUIC.geoffkizer commentedon Oct 14, 2020
I think from a usability point of view, returning null from AcceptConnection or AcceptStream when a local close has happened is the best behavior, even though it's different than the behavior of Socket.Accept.
AcceptStream can also fail because of a remote close, either graceful or abortive; I'm less sure what should happen in this case. I think you can make an argument that graceful remote close should result in null from AcceptStream. Abortive remote close probably shouldn't.
geoffkizer commentedon Oct 14, 2020
Agreed, this is a good issue.
geoffkizer commentedon Nov 17, 2020
I think QuicStream issues are covered under #756.
Let's use this issue specifically for QuicConnection and QuicListener. (Or maybe we should open issues for each...)
Edit: I do think the AcceptConnection and AcceptStream issues are very similar; we should be consistent here. I think the only other issue then, is Dispose during Connect. Am I missing anything here?
ManickaP commentedon Mar 4, 2021
Meeting notes:
ObjectDisposedExceptionin both scenarios: Read/etc called before Dispose, Read/etc called after Dispose. This will save us a race condition.ManickaP commentedon Jul 15, 2021
Triage: we should investigate for 6.0 whether we're not hanging in case of a disposal and whether we're propagating a correct exception outside of the calls in flight.
Some of this is already fixed.
ManickaP commentedon Jun 14, 2022
Closing as this is covered by #67560 #68902 and #70669