Skip to content

Commit

Permalink
Prevent reverting SendState from Aborted/ConnectionClosed back to sen…
Browse files Browse the repository at this point in the history 
…ding state within Send* methods.
  • Loading branch information
@ManickaP
ManickaP authored and github-actions committed Aug 24, 2021
1 parent ab8b491 commit a28a614
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 29 deletions.
8 changes: 5 additions & 3 deletions src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public async Task RequestSentResponseDisposed_ThrowsOnServer()

Stopwatch sw = Stopwatch.StartNew();
bool hasFailed = false;
while (sw.Elapsed < TestHelper.PassingTestTimeout)
while (sw.Elapsed < TimeSpan.FromSeconds(15))
{
try
{
Expand Down Expand Up @@ -371,6 +371,7 @@ public async Task RequestSentResponseDisposed_ThrowsOnServer()

// We haven't finished reading the whole respose, but we're disposing it, which should turn into an exception on the server-side.
response.Dispose();
await serverTask;
});

await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000);
Expand All @@ -392,7 +393,7 @@ public async Task RequestSendingResponseDisposed_ThrowsOnServer()

Stopwatch sw = Stopwatch.StartNew();
bool hasFailed = false;
while (sw.Elapsed < TestHelper.PassingTestTimeout)
while (sw.Elapsed < TimeSpan.FromSeconds(15))
{
try
{
Expand Down Expand Up @@ -425,6 +426,7 @@ public async Task RequestSendingResponseDisposed_ThrowsOnServer()

// We haven't finished sending the whole request, but we're disposing the response, which should turn into an exception on the server-side.
response.Dispose();
await serverTask;
});

await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000);
Expand Down Expand Up @@ -996,7 +998,7 @@ public async Task StatusCodes_ReceiveSuccess(HttpStatusCode statusCode, bool qpa
VersionPolicy = HttpVersionPolicy.RequestVersionExact
};
HttpResponseMessage response = await client.SendAsync(request).WaitAsync(TimeSpan.FromSeconds(10));

Assert.Equal(statusCode, response.StatusCode);

await serverTask;
Expand Down
40 changes: 14 additions & 26 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ internal override async ValueTask WriteAsync(ReadOnlySequence<byte> buffers, boo
{
ThrowIfDisposed();

using CancellationTokenRegistration registration = HandleWriteStartState(cancellationToken);
using CancellationTokenRegistration registration = HandleWriteStartState(buffers.IsEmpty, cancellationToken);

await SendReadOnlySequenceAsync(buffers, endStream ? QUIC_SEND_FLAGS.FIN : QUIC_SEND_FLAGS.NONE).ConfigureAwait(false);

Expand All @@ -281,7 +281,7 @@ internal override async ValueTask WriteAsync(ReadOnlyMemory<ReadOnlyMemory<byte>
{
ThrowIfDisposed();

using CancellationTokenRegistration registration = HandleWriteStartState(cancellationToken);
using CancellationTokenRegistration registration = HandleWriteStartState(buffers.IsEmpty, cancellationToken);

await SendReadOnlyMemoryListAsync(buffers, endStream ? QUIC_SEND_FLAGS.FIN : QUIC_SEND_FLAGS.NONE).ConfigureAwait(false);

Expand All @@ -292,20 +292,20 @@ internal override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, bool e
{
ThrowIfDisposed();

using CancellationTokenRegistration registration = HandleWriteStartState(cancellationToken);
using CancellationTokenRegistration registration = HandleWriteStartState(buffer.IsEmpty, cancellationToken);

await SendReadOnlyMemoryAsync(buffer, endStream ? QUIC_SEND_FLAGS.FIN : QUIC_SEND_FLAGS.NONE).ConfigureAwait(false);

HandleWriteCompletedState();
}

private CancellationTokenRegistration HandleWriteStartState(CancellationToken cancellationToken)
private CancellationTokenRegistration HandleWriteStartState(bool emptyBuffer, CancellationToken cancellationToken)
{
if (_state.SendState == SendState.Closed)
{
throw new InvalidOperationException(SR.net_quic_writing_notallowed);
}
else if ( _state.SendState == SendState.Aborted)
if (_state.SendState == SendState.Aborted)
{
if (_state.SendErrorCode != -1)
{
Expand Down Expand Up @@ -363,10 +363,14 @@ private CancellationTokenRegistration HandleWriteStartState(CancellationToken ca

throw new OperationCanceledException(SR.net_quic_sending_aborted);
}
else if (_state.SendState == SendState.ConnectionClosed)
if (_state.SendState == SendState.ConnectionClosed)
{
throw GetConnectionAbortedException(_state);
}

// Change the state in the same lock where we check for final states to prevent coming back from Aborted/ConnectionClosed.
Debug.Assert(_state.SendState != SendState.Pending);
_state.SendState = emptyBuffer ? SendState.Finished : SendState.Pending;
}

return registration;
Expand Down Expand Up @@ -632,7 +636,10 @@ internal override void Shutdown()

lock (_state)
{
_state.SendState = SendState.Finished;
if (_state.SendState < SendState.Finished)
{
_state.SendState = SendState.Finished;
}
}

// it is ok to send shutdown several times, MsQuic will ignore it
Expand Down Expand Up @@ -1157,12 +1164,6 @@ private unsafe ValueTask SendReadOnlyMemoryAsync(
ReadOnlyMemory<byte> buffer,
QUIC_SEND_FLAGS flags)
{
lock (_state)
{
Debug.Assert(_state.SendState != SendState.Pending);
_state.SendState = buffer.IsEmpty ? SendState.Finished : SendState.Pending;
}

if (buffer.IsEmpty)
{
if ((flags & QUIC_SEND_FLAGS.FIN) == QUIC_SEND_FLAGS.FIN)
Expand Down Expand Up @@ -1211,13 +1212,6 @@ private unsafe ValueTask SendReadOnlySequenceAsync(
ReadOnlySequence<byte> buffers,
QUIC_SEND_FLAGS flags)
{

lock (_state)
{
Debug.Assert(_state.SendState != SendState.Pending);
_state.SendState = buffers.IsEmpty ? SendState.Finished : SendState.Pending;
}

if (buffers.IsEmpty)
{
if ((flags & QUIC_SEND_FLAGS.FIN) == QUIC_SEND_FLAGS.FIN)
Expand Down Expand Up @@ -1281,12 +1275,6 @@ private unsafe ValueTask SendReadOnlyMemoryListAsync(
ReadOnlyMemory<ReadOnlyMemory<byte>> buffers,
QUIC_SEND_FLAGS flags)
{
lock (_state)
{
Debug.Assert(_state.SendState != SendState.Pending);
_state.SendState = buffers.IsEmpty ? SendState.Finished : SendState.Pending;
}

if (buffers.IsEmpty)
{
if ((flags & QUIC_SEND_FLAGS.FIN) == QUIC_SEND_FLAGS.FIN)
Expand Down

0 comments on commit a28a614

Please sign in to comment.