Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Triggering IConnectionLifetimeNotificationFeature on GoAway #43431

Merged
merged 1 commit into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,9 @@ private Task ProcessGoAwayFrameAsync()
throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamIdNotZero(_incomingFrame.Type), Http2ErrorCode.PROTOCOL_ERROR);
}

// StopProcessingNextRequest must be called before RequestClose to ensure it's considered client initiated.
StopProcessingNextRequest(serverInitiated: false);
ladeak marked this conversation as resolved.
Show resolved Hide resolved
_context.ConnectionFeatures.Get<IConnectionLifetimeNotificationFeature>()?.RequestClose();

return Task.CompletedTask;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ internal abstract class Http3ControlStream : IHttp3Stream, IThreadPoolWorkItem
private readonly Http3RawFrame _incomingFrame = new Http3RawFrame();
private volatile int _isClosed;
private long _headerType;
private int _gracefulCloseInitiator;
private readonly object _completionLock = new();

private bool _haveReceivedSettingsFrame;
Expand Down Expand Up @@ -387,6 +386,10 @@ private ValueTask ProcessGoAwayFrameAsync()
{
EnsureSettingsFrame(Http3FrameType.GoAway);

// StopProcessingNextRequest must be called before RequestClose to ensure it's considered client initiated.
_context.Connection.StopProcessingNextRequest(serverInitiated: false);
ladeak marked this conversation as resolved.
Show resolved Hide resolved
_context.ConnectionContext.Features.Get<IConnectionLifetimeNotificationFeature>()?.RequestClose();

// https://quicwg.org/base-drafts/draft-ietf-quic-http.html#name-goaway
// PUSH is not implemented so nothing to do.

Expand Down Expand Up @@ -432,19 +435,6 @@ private void EnsureSettingsFrame(Http3FrameType frameType)
}
}

public void StopProcessingNextRequest()
=> StopProcessingNextRequest(serverInitiated: true);

public void StopProcessingNextRequest(bool serverInitiated)
{
var initiator = serverInitiated ? GracefulCloseInitiator.Server : GracefulCloseInitiator.Client;

if (Interlocked.CompareExchange(ref _gracefulCloseInitiator, initiator, GracefulCloseInitiator.None) == GracefulCloseInitiator.None)
{
Input.CancelPendingRead();
}
}

/// <summary>
/// Used to kick off the request processing loop by derived classes.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ public void OnHeartbeat(Action<object> action, object state)

public void RequestClose()
{
throw new NotImplementedException();
ConnectionClosingCts.Cancel();
ladeak marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Globalization;
using System.Net.Http;
using System.Net.Http.HPack;
using System.Net.Security;
using System.Reflection;
using System.Security.Authentication;
using System.Text;
Expand Down Expand Up @@ -3855,6 +3854,19 @@ public async Task GOAWAY_Received_ConnectionStops()
await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false);
}

[Fact]
public async Task GOAWAY_Received_ConnectionLifetimeNotification_Cancelled()
{
await InitializeConnectionAsync(_noopApplication, addKestrelFeatures: true);
var lifetime = _connection.ConnectionFeatures.Get<IConnectionLifetimeNotificationFeature>();
Assert.False(lifetime.ConnectionClosedRequested.IsCancellationRequested);

await SendGoAwayAsync();

Assert.True(lifetime.ConnectionClosedRequested.IsCancellationRequested);
await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false);
}

[Fact]
public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,12 +452,14 @@ protected void CreateConnection()

_pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions);

var features = new FeatureCollection();
_mockConnectionContext.Setup(x => x.Features).Returns(features);
var httpConnectionContext = TestContextFactory.CreateHttpConnectionContext(
serviceContext: _serviceContext,
connectionContext: _mockConnectionContext.Object,
transport: _pair.Transport,
memoryPool: _memoryPool,
connectionFeatures: new FeatureCollection(),
connectionFeatures: features,
timeoutControl: _mockTimeoutControl.Object);

_connection = new Http2Connection(httpConnectionContext);
Expand Down Expand Up @@ -500,12 +502,16 @@ public void OnStreamCompleted(Http2Stream stream)
}
}

protected void InitializeConnectionWithoutPreface(RequestDelegate application)
protected void InitializeConnectionWithoutPreface(RequestDelegate application, bool addKestrelFeatures = false)
{
if (_connection == null)
{
CreateConnection();
}
if (addKestrelFeatures)
{
AddKestrelConnection();
}

var connectionTask = _connection.ProcessRequestsAsync(new DummyApplication(application));

Expand All @@ -525,9 +531,9 @@ async Task CompletePipeOnTaskCompletion()
_connectionTask = CompletePipeOnTaskCompletion();
}

protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 4, bool expectedWindowUpdate = true)
protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 4, bool expectedWindowUpdate = true, bool addKestrelFeatures = false)
{
InitializeConnectionWithoutPreface(application);
InitializeConnectionWithoutPreface(application, addKestrelFeatures);

// Lose xUnit's AsyncTestSyncContext so middleware always runs inline for better determinism.
await ThreadPoolAwaitable.Instance;
Expand All @@ -554,6 +560,17 @@ await ExpectAsync(Http2FrameType.SETTINGS,
withStreamId: 0);
}

protected void AddKestrelConnection()
{
new KestrelConnection<BaseConnectionContext>(
0,
_serviceContext,
new TransportConnectionManager(_serviceContext.ConnectionManager),
_ => throw new NotImplementedException($"{nameof(_connection.ProcessRequestsAsync)} should invoked instead - hence transport connection manager does not have the connection registered."),
_mockConnectionContext.Object,
new KestrelTrace(_serviceContext.LoggerFactory));
}

protected Task StartStreamAsync(int streamId, IEnumerable<KeyValuePair<string, string>> headers, bool endStream, bool flushFrame = true)
{
var writableBuffer = _pair.Application.Output;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,30 @@ await Http3Api.WaitForConnectionErrorAsync<Http3ConnectionErrorException>(
}

[Fact]
public async Task ControlStream_ServerToClient_Closes_ConnectionError()
public async Task GOAWAY_TriggersLifetimeNotification_ConnectionClosedRequested()
{
var completionSource = new TaskCompletionSource();
await Http3Api.InitializeConnectionAsync(_noopApplication);

var controlStream = await Http3Api.CreateControlStream(id: 0);
await controlStream.SendSettingsAsync(new List<Http3PeerSetting>());
var lifetime = Http3Api.MultiplexedConnectionContext.Features.Get<IConnectionLifetimeNotificationFeature>();
lifetime.ConnectionClosedRequested.Register(() => completionSource.TrySetResult());
Assert.False(lifetime.ConnectionClosedRequested.IsCancellationRequested);

await controlStream.SendGoAwayAsync(streamId: 0, false);

await completionSource.Task.DefaultTimeout();
Assert.True(lifetime.ConnectionClosedRequested.IsCancellationRequested);

// Trigger server shutdown.
Http3Api.CloseServerGracefully();

await Http3Api.WaitForConnectionStopAsync(0, true, expectedErrorCode: Http3ErrorCode.NoError);
}

[Fact]
public async Task ControlStream_ServerToClient_ErrorInitializing_ConnectionError()
{
var now = _serviceContext.MockSystemClock.UtcNow;

Expand Down