icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
92 stars 14 forks source link

Align Slic connection dispose behavior with Quic #2225

Open bernardnormier opened 1 year ago

bernardnormier commented 1 year ago

See #2183.

In general, I think Slic should behave like Quic whenever doable even if we think the Quic behavior is not the best. We can change Slic and keep in in sync with Quic over time; we have little control over .NET's Quic implementation.

In this particular case, SlicConnection.DisposeAsync should send a Close frame like Quic (or Quic will soon). The application error code would be a "default error code" set in the Slic options.

bentoi commented 1 year ago
    Really: the "accept requests loop" (or accept requests task) disposed the transport connection while this CloseAsync was in progress.

And presumably, transportConnection.DisposeAsync does not wait for an outstanding CloseAsync to complete: it aborts it. Is this the correct semantics for multiplexed connection?

See also https://github.com/dotnet/runtime/issues/78641#issuecomment-1322506759

_Originally posted by @bernardnormier in https://github.com/zeroc-ice/icerpc-csharp/pull/2289#discussion_r1047697582_

bernardnormier commented 1 year ago

Microsoft does not describe what they want to do when DisposeAsync is called while CloseAsync is in progress. Presumably it could be: a) wait for the CloseAsync to complete b) somehow abort the CloseAsync and wait for this aborted CloseAsync to complete

We should ask Manicka on this issue 78641.

And once we know, we do the same in Slic.

bentoi commented 1 year ago

The protocol connection disposal is now abortive like Slic. If we allow the transport DisposeAsync to be graceful, would there still be a good reason for IProtocolConnection.DisposeAsync to not be graceful like Quic (graceful if ShutdownAsync didn't previous fail or was not called)?

Or should we push for a silent close implementation of QuicConnetion.DisposeAsync?

I'm moving to 0.2 for now.

bentoi commented 5 months ago

See https://github.com/dotnet/runtime/pull/96807 that fixes the issue I reported. We need to review how these changes are relevant with this issue.

For instance, SlicConnection.DisposeAsync is already abortive so Quic is now aligned with Slic. It's not clear however, if Quic and Slic behave the same when DisposeAsync is called while a CloseAsync call is in progress, it's likely.