Send path can result in empty send

[nibanks]

Describe the bug

During a SpinQuic run, it seems an alloc failure eventually led to a send with no actual data buffered. Here's the callstack:

0a 000000dc`6e8fd8c0 00007ffa`d39b64c0 ntdll!KiUserExceptionDispatcher+0x2e
0b 000000dc`6e8fdfd0 00007ffa`d38bf89f msquic!CxPlatSocketSend+0xf0 [D:\a\1\msquic\src\platform\datapath_winuser.c @ 3806] 
0c 000000dc`6e8fe100 00007ffa`d39a521b msquic!QuicBindingSend+0x4ef [D:\a\1\msquic\src\core\binding.c @ 1776] 
0d 000000dc`6e8fe510 00007ffa`d39a4a70 msquic!QuicPacketBuilderSendBatch+0x44b [D:\a\1\msquic\src\core\packet_builder.c @ 973] 
0e 000000dc`6e8fe730 00007ffa`d39473d8 msquic!QuicPacketBuilderFinalize+0x1a20 [D:\a\1\msquic\src\core\packet_builder.c @ 923] 
0f 000000dc`6e8fed10 00007ffa`d38e876b msquic!QuicSendFlush+0x12e8 [D:\a\1\msquic\src\core\send.c @ 1226] 
10 000000dc`6e8ff5d0 00007ffa`d39262ec msquic!QuicConnDrainOperations+0x67b [D:\a\1\msquic\src\core\connection.c @ 6755] 
11 000000dc`6e8ff830 00007ffa`d392368e msquic!QuicWorkerProcessConnection+0x59c [D:\a\1\msquic\src\core\worker.c @ 487] 
12 000000dc`6e8ffae0 00007ffa`d095bf14 msquic!QuicWorkerThread+0x34e [D:\a\1\msquic\src\core\worker.c @ 579] 
13 000000dc`6e8ffd10 00007ffb`047a54e0 clang_rt_asan_dbg_dynamic_x86_64!_asan_wrap_GlobalSize+0x5924a
14 000000dc`6e8ffd50 00007ffb`04e005fb kernel32!BaseThreadInitThunk+0x10
15 000000dc`6e8ffd80 00000000`00000000 ntdll!RtlUserThreadStart+0x2b

Dump/symbols: spinquic.zip

[thhous-msft]

Turns out this bug is not actually an out of memory bug, but an out of send allowance bug. This occurs if the send loop runs once, can put some data but not all into the send packet, sends that packet, and then tries to place the rest of the data in the same loop in another packet. This causes an empty send to occur, because it doesn't have enough allowance to fill the data, but we've already sent a datagram, so the packet builder code attempts to send the empty packet.

FinalQuicPacket = FlushBatchedDatagrams && (Builder->TotalCountDatagrams != 0);

That line should probably be changed to

        FinalQuicPacket =
            FlushBatchedDatagrams &&
            (Builder->TotalCountDatagrams != 0) &&
            Builder->Datagram != NULL;

But we need some testing to make sure that is correct.

Logs.zip

[thhous-msft]

The assert has been removed, as it was benign. However at some point it would be nice to fix the empty send, as its just wasted work. The fix suggested above is not correct. It will require a much deeper investigation.