diff --git a/src/core/stream_send.c b/src/core/stream_send.c index 042e6c7028..49e70d0f07 100644 --- a/src/core/stream_send.c +++ b/src/core/stream_send.c @@ -47,6 +47,16 @@ QuicStreamCompleteSendRequest( _In_ BOOLEAN PreviouslyPosted ); +// +// Enqueues a SendRequest from the temporary queue to the actual queue. +// +_IRQL_requires_max_(PASSIVE_LEVEL) +void +QuicStreamEnqueueSendRequest( + _In_ QUIC_STREAM* Stream, + _Inout_ QUIC_SEND_REQUEST* SendRequest + ); + #if DEBUG _IRQL_requires_max_(PASSIVE_LEVEL) @@ -249,6 +259,25 @@ QuicStreamSendShutdown( } Stream->Flags.LocalCloseResetReliable = TRUE; Stream->SendShutdownErrorCode = ErrorCode; + + while (ApiSendRequests != NULL) { + // + // App queued some data around the same time it called shutdown. Bad App! + // Enqueue all the stuff in ApiSendRequests to SendRequests. + // + QUIC_SEND_REQUEST* SendRequest = ApiSendRequests; + ApiSendRequests = ApiSendRequests->Next; + SendRequest->Next = NULL; + QuicStreamEnqueueSendRequest(Stream, SendRequest); + + if (Stream->Connection->Settings.SendBufferingEnabled) { + QuicSendBufferFill(Stream->Connection); + } + + CXPLAT_DBG_ASSERT(Stream->SendRequests != NULL); + QuicStreamSendDumpState(Stream); + } + // // Queue up a RESET RELIABLE STREAM frame to be sent. We will clear up any flags later. // @@ -514,6 +543,63 @@ QuicStreamSendBufferRequest( return QUIC_STATUS_SUCCESS; } +_IRQL_requires_max_(PASSIVE_LEVEL) +void +QuicStreamEnqueueSendRequest( + _In_ QUIC_STREAM* Stream, + _Inout_ QUIC_SEND_REQUEST* SendRequest + ) +{ + Stream->Connection->SendBuffer.PostedBytes += SendRequest->TotalLength; + + // + // Queue up the send request. + // + + QuicStreamRemoveOutFlowBlockedReason(Stream, QUIC_FLOW_BLOCKED_APP); + + SendRequest->StreamOffset = Stream->QueuedSendOffset; + Stream->QueuedSendOffset += SendRequest->TotalLength; + + if (SendRequest->Flags & QUIC_SEND_FLAG_ALLOW_0_RTT && + Stream->Queued0Rtt == SendRequest->StreamOffset) { + Stream->Queued0Rtt = Stream->QueuedSendOffset; + } + + // + // The bookmarks are set to NULL once the entire request queue is + // consumed. So if a bookmark is NULL here, we should set it to + // point to the new request at the end of the queue, to prevent + // a subsequent search over the entire queue in the code that + // uses the bookmark. + // + if (Stream->SendBookmark == NULL) { + Stream->SendBookmark = SendRequest; + } + if (Stream->SendBufferBookmark == NULL) { + // + // If we have no SendBufferBookmark, that must mean we have no + // unbuffered send requests queued currently. + // + CXPLAT_DBG_ASSERT( + Stream->SendRequests == NULL || + !!(Stream->SendRequests->Flags & QUIC_SEND_FLAG_BUFFERED)); + Stream->SendBufferBookmark = SendRequest; + } + + *Stream->SendRequestsTail = SendRequest; + Stream->SendRequestsTail = &SendRequest->Next; + + QuicTraceLogStreamVerbose( + SendQueued, + Stream, + "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", + SendRequest, + SendRequest->TotalLength, + SendRequest->StreamOffset, + SendRequest->Flags); +} + _IRQL_requires_max_(PASSIVE_LEVEL) void QuicStreamSendFlush( @@ -545,54 +631,7 @@ QuicStreamSendFlush( continue; } - Stream->Connection->SendBuffer.PostedBytes += SendRequest->TotalLength; - - // - // Queue up the send request. - // - - QuicStreamRemoveOutFlowBlockedReason(Stream, QUIC_FLOW_BLOCKED_APP); - - SendRequest->StreamOffset = Stream->QueuedSendOffset; - Stream->QueuedSendOffset += SendRequest->TotalLength; - - if (SendRequest->Flags & QUIC_SEND_FLAG_ALLOW_0_RTT && - Stream->Queued0Rtt == SendRequest->StreamOffset) { - Stream->Queued0Rtt = Stream->QueuedSendOffset; - } - - // - // The bookmarks are set to NULL once the entire request queue is - // consumed. So if a bookmark is NULL here, we should set it to - // point to the new request at the end of the queue, to prevent - // a subsequent search over the entire queue in the code that - // uses the bookmark. - // - if (Stream->SendBookmark == NULL) { - Stream->SendBookmark = SendRequest; - } - if (Stream->SendBufferBookmark == NULL) { - // - // If we have no SendBufferBookmark, that must mean we have no - // unbuffered send requests queued currently. - // - CXPLAT_DBG_ASSERT( - Stream->SendRequests == NULL || - !!(Stream->SendRequests->Flags & QUIC_SEND_FLAG_BUFFERED)); - Stream->SendBufferBookmark = SendRequest; - } - - *Stream->SendRequestsTail = SendRequest; - Stream->SendRequestsTail = &SendRequest->Next; - - QuicTraceLogStreamVerbose( - SendQueued, - Stream, - "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", - SendRequest, - SendRequest->TotalLength, - SendRequest->StreamOffset, - SendRequest->Flags); + QuicStreamEnqueueSendRequest(Stream, SendRequest); if (SendRequest->Flags & QUIC_SEND_FLAG_START && !Stream->Flags.Started) { // diff --git a/src/generated/linux/stream_send.c.clog.h b/src/generated/linux/stream_send.c.clog.h index 2c4508c60d..e93319b7b1 100644 --- a/src/generated/linux/stream_send.c.clog.h +++ b/src/generated/linux/stream_send.c.clog.h @@ -87,13 +87,13 @@ tracepoint(CLOG_STREAM_SEND_C, IndicateSendComplete , arg1, arg3);\ // Decoder Ring for SendQueued // [strm][%p] Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x) // QuicTraceLogStreamVerbose( - SendQueued, - Stream, - "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", - SendRequest, - SendRequest->TotalLength, - SendRequest->StreamOffset, - SendRequest->Flags); + SendQueued, + Stream, + "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", + SendRequest, + SendRequest->TotalLength, + SendRequest->StreamOffset, + SendRequest->Flags); // arg1 = arg1 = Stream = arg1 // arg3 = arg3 = SendRequest = arg3 // arg4 = arg4 = SendRequest->TotalLength = arg4 diff --git a/src/generated/linux/stream_send.c.clog.h.lttng.h b/src/generated/linux/stream_send.c.clog.h.lttng.h index f37ae2c3cb..3bb19bd68c 100644 --- a/src/generated/linux/stream_send.c.clog.h.lttng.h +++ b/src/generated/linux/stream_send.c.clog.h.lttng.h @@ -70,13 +70,13 @@ TRACEPOINT_EVENT(CLOG_STREAM_SEND_C, IndicateSendComplete, // Decoder Ring for SendQueued // [strm][%p] Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x) // QuicTraceLogStreamVerbose( - SendQueued, - Stream, - "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", - SendRequest, - SendRequest->TotalLength, - SendRequest->StreamOffset, - SendRequest->Flags); + SendQueued, + Stream, + "Send Request [%p] queued with %llu bytes at offset %llu (flags 0x%x)", + SendRequest, + SendRequest->TotalLength, + SendRequest->StreamOffset, + SendRequest->Flags); // arg1 = arg1 = Stream = arg1 // arg3 = arg3 = SendRequest = arg3 // arg4 = arg4 = SendRequest->TotalLength = arg4 diff --git a/src/test/lib/DataTest.cpp b/src/test/lib/DataTest.cpp index 4eac48b7ee..cd7ddde2dd 100644 --- a/src/test/lib/DataTest.cpp +++ b/src/test/lib/DataTest.cpp @@ -3455,6 +3455,7 @@ QuicTestStreamReliableResetMultipleSends( SendContext send3 = {FALSE, 0}; SendContext send4 = {FALSE, 0}; SendContext send5 = {FALSE, 0}; + SendContext send6 = {FALSE, 0}; TEST_QUIC_SUCCEEDED(Stream.Send(&SendBuffer, 1, QUIC_SEND_FLAG_DELAY_SEND, &send1)); TEST_QUIC_SUCCEEDED(Stream.Send(&SendBuffer, 1, QUIC_SEND_FLAG_DELAY_SEND, &send2)); TEST_QUIC_SUCCEEDED(Stream.Send(&SendBuffer, 1, QUIC_SEND_FLAG_DELAY_SEND, &send3)); @@ -3463,8 +3464,15 @@ QuicTestStreamReliableResetMultipleSends( TEST_QUIC_SUCCEEDED(Stream.SetReliableOffset(RELIABLE_SIZE_MULTI_SENDS)); const QUIC_UINT62 AbortShutdownErrorCode = 0x696969696969; - TEST_QUIC_SUCCEEDED(Stream.Shutdown(AbortShutdownErrorCode)); // Queues up a shutdown operation. - // Should behave similar to QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, with some restrictions. + TEST_QUIC_SUCCEEDED(Stream.Shutdown(AbortShutdownErrorCode)); + + // + // An app shouldn't be sending after it just called shutdown, but we want to make sure this + // doesn't cause a memory leak or other problems. + // + Stream.Send(&SendBuffer, 1, QUIC_SEND_FLAG_NONE, &send6); // This may or may not succeed (race condition). + + TEST_TRUE(Context.ClientStreamShutdownComplete.WaitTimeout(TestWaitTimeout)); TEST_TRUE(Context.ServerStreamShutdownComplete.WaitTimeout(TestWaitTimeout)); TEST_TRUE(Context.ReceivedBufferSize >= RELIABLE_SIZE_MULTI_SENDS);