Resumable connection and explicit ShutdownAsync/CloseAsync calls

[bentoi]

Related to #889

What should be the behavior of the following code?

await using var connection = new Connection(new ConnectionOptions() 
    { 
        IsResumable = true;
        RemoteEndpoint = ...
    });
var prx = ServicePrx.FromConnection(connection);
await prx.IcePingAsync()
await connection.ShutdownAsync();
await prx.IcePingAsync();

Should the second IcePingAsync:

• fail because the connection is considered non-resumable when the application explicitly shuts it down (by calling ShutdownAsync, CloseAsync or DisposeAsync),
• succeed because the connection can be resumed after the application called ShutdownAsync or CloseAsync. The connection is no longer resumable only if DisposeAsync is called.

We implement the second option but I'm thinking the first one would be better given that DisposeAsync semantics are the same as calling ShutdownAsync("", new CancellationToken(canceled: true)); (like Server). In other words, explicitly shutting down, closing or disposing the connection should make the connection non-resumable.

The connection ClosedEvent and the new ShutdownCompletion task would only be called/completed once the application call to ShutdownAsync, CloseAsync or DisposeAsync complete.

[bernardnormier]

This sounds fine.

[pepone]

I have slightly preference for use StateChanged , Changed suffix seems more common with .NET Events

https://github.com/dotnet/runtime/search?p=1&q=Changed
https://github.com/dotnet/runtime/search?p=1&q=Updated

[bernardnormier]

Somewhat related: I find the Close vs Shutdown distinction very confusing.

If close = non-graceful shutdown, should we just add a bool parameter to ShutdownAsync?

[bentoi]

I was thinking of renaming CloseAsync to AbortAsync to be more explicit. I can live with a bool if you prefer (bool abort?). Using abort raises the question of state names though, Aborting and Aborted instead of Closing, Closed?

What I don't really like is that DisposeAsync behaves like a "speedy" graceful shutdown, I would expect it to behave like an abort of the connection.

[bernardnormier]

As far as I can tell, CloseAsync is async only because INetworkConnection is IAsyncDisposable but not Disposable and we don't distinguish between graceful and non-graceful closure for INetworkConnection.

Could we either make INetworkConnection Disposable (and as a result Close non-async) or add graceful vs non-graceful closure of an INetworkConnection where the non-graceful closure is sync?

I find Abort + Shutdown a little bit clearer than Close + Shutdown, however I think a single term would be even better, like:
state = shutting down (graceful)
state = shutting down (abort)

vs

state = shutting down
state = aborting

[bentoi]

we don't distinguish between graceful and non-graceful closure for INetworkConnection.

Yes, the ice and icerpc protocol implement graceful closure and close the network transport connection without making any assumption on the transport closure implementation.

Could we either make INetworkConnection Disposable

It's a bit like the PipeReader/Writer complete methods. Some transport implementations might require async, others don't.

In general higher level transport protocols such as SSL or WebSocket have their own shutdown logic which is async (TCP also has shutdown write actually).

However, we prefer to implement our own shutdown logic with ice/icerpc to guarantee at-most-once retry semantics.

HttpClient also doesn't use it, see dotnet/runtime#44660

There's also more discussions on this here: dotnet/runtime#43290 As you can see, lots of discussions there :).

I think the simplest is to keep the transport async disposable and try to ensure that the transport connection is closed with the recommended sequence (if the language API allows it!). And even if don't really care because we use our own graceful shutdown logic to ensure it works regardless of what the underlying transport implements.

[bernardnormier]

Say we use a transport which provides only an async network connection closure, say CloseAsync.

And we implement XxxNetworkConnection.Dispose with _ => _handle.CloseAsync().AsTask();
(we don't await it)

What would be the downside? That we may lose some CloseAsync() exceptions?

[bentoi]

As of today, it makes no differences (especially since we don't care about sending SSL close_notify).

However I have no idea if tomorrow a transport will require an async graceful closure for which it will be desirable to wait for the closure (for the application implementation purpose).

For sure, we don't take a great risk at having a synchronous network connection dispose method.

[bernardnormier]

@bentoi, can we close this issue?