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.

Sign up for GitHub

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

Shutdown Stream on Send/Start Failure #3637

Merged
merged 6 commits into from
May 19, 2023
Merged

Shutdown Stream on Send/Start Failure #3637

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0f9c34
Shutdown Stream on Send/Start Failure
nibanks May 17, 2023
cd5447e
Kill the connection on OOM
nibanks May 17, 2023
9197a26
Don't forget to queue
nibanks May 17, 2023
1006677
Trigger transport shutdown
nibanks May 17, 2023
f311c73
Build fixes
nibanks May 18, 2023
3f96f59
Add back check for downlevel tests
nibanks May 18, 2023
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
24 changes: 18 additions & 6 deletions src/core/api.c
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May need to add 3 unit test cases to cover the following:

  • If StreamSend returns SUCCESS, SEND_COMPLETE must be indicated exactly-once
  • If StreamSend returns FAIL, SEND_COMPLETE must not be indicated.
  • If StreamSend(QUIC_SEND_FLAG_START) returns SUCCESS, STREAM_SHUTDOWN_COMPLETE must be indicated exactly-once.

Original file line number Diff line number Diff line change
Expand Up @@ -1039,11 +1039,6 @@ MsQuicStreamSend(
(Connection->WorkerThreadID == CxPlatCurThreadID()) ||
!Connection->State.HandleClosed);

if (Connection->State.ClosedRemotely) {
Status = QUIC_STATUS_ABORTED;
goto Exit;
}

TotalLength = 0;
for (uint32_t i = 0; i < BufferCount; ++i) {
TotalLength += Buffers[i].Length;
Expand Down Expand Up @@ -1125,12 +1120,29 @@ MsQuicStreamSend(
} else if (QueueOper) {
Oper = QuicOperationAlloc(Connection->Worker, QUIC_OPER_TYPE_API_CALL);
if (Oper == NULL) {
Status = QUIC_STATUS_OUT_OF_MEMORY;
QuicTraceEvent(
AllocFailure,
"Allocation of '%s' failed. (%llu bytes)",
"STRM_SEND operation",
0);
//
// We can't fail the send at this point, because we're already queued
// the send above. So instead, we're just going to abort the whole
// connection.
//
if (InterlockedCompareExchange16(
(short*)&Connection->BackUpOperUsed, 1, 0) != 0) {
goto Exit; // It's already started the shutdown.
}
Oper = &Connection->BackUpOper;
Oper->FreeAfterProcess = FALSE;
Oper->Type = QUIC_OPER_TYPE_API_CALL;
Oper->API_CALL.Context = &Connection->BackupApiContext;
Oper->API_CALL.Context->Type = QUIC_API_TYPE_CONN_SHUTDOWN;
Oper->API_CALL.Context->CONN_SHUTDOWN.Flags = QUIC_CONNECTION_SHUTDOWN_FLAG_SILENT;
Oper->API_CALL.Context->CONN_SHUTDOWN.ErrorCode = 0;
nibanks marked this conversation as resolved.
Show resolved Hide resolved
Oper->API_CALL.Context->CONN_SHUTDOWN.RegistrationShutdown = FALSE;
QuicConnQueueOper(Connection, Oper);
goto Exit;
}
Oper->API_CALL.Context->Type = QUIC_API_TYPE_STRM_SEND;
Expand Down
2 changes: 1 addition & 1 deletion src/core/stream_send.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ QuicStreamSendFlush(
if (Start) {
(void)QuicStreamStart(
Stream,
QUIC_STREAM_START_FLAG_IMMEDIATE,
QUIC_STREAM_START_FLAG_IMMEDIATE | QUIC_STREAM_START_FLAG_SHUTDOWN_ON_FAIL,
FALSE);
}

Expand Down