microsoft / msquic

Cross-platform, C implementation of the IETF QUIC protocol, exposed to C, C++, C# and Rust.
MIT License
3.92k stars 525 forks source link

Basic/WithRebindPaddingArgs.RebindAddrPadded failed with assert #4330

Open ami-GS opened 1 month ago

ami-GS commented 1 month ago

Describe the bug

Windows got assert by Basic/WithRebindPaddingArgs.RebindAddrPadded/*

[Microsoft-Quic][ lib] ASSERT, D:\a\msquic\msquic\src\core\send.c:957 - Builder->Metadata->FrameCount > PrevFrameCount || RanOutOfRoom || (CxPlatGetAllocFailDenominator() != 0).

image

Affected OS

Additional OS information

No response

MsQuic version

main

Steps taken to reproduce bug

needs automation

Expected behavior

pass

Actual outcome

[Microsoft-Quic][ lib] ASSERT, D:\a\msquic\msquic\src\core\send.c:957 - Builder->Metadata->FrameCount > PrevFrameCount || RanOutOfRoom || (CxPlatGetAllocFailDenominator() != 0).

Additional details

By comparing passed case (with Ubuntu),

image These are arrived at once with second QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED indication after the first QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED indication.

This is first indication image

This is second indication image

This is the assert after the second indication [Microsoft-Quic][ lib] ASSERT, D:\a\msquic\msquic\src\core\send.c:957 - Builder->Metadata->FrameCount > PrevFrameCount || RanOutOfRoom || (CxPlatGetAllocFailDenominator() != 0).

ami-GS commented 1 month ago

Maybe, the cause of 2 QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED indication is that these PING frames are arrived after unregistering packet hook? so QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED happens to change back to original peer address?

image

Then the second QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED happens with receiving PING frames

Not sure about the ASSERT yet

nibanks commented 1 month ago

This is the assert after the second indication [Microsoft-Quic][ lib] ASSERT, D:\a\msquic\msquic\src\core\send.c:957 - Builder->Metadata->FrameCount > PrevFrameCount || RanOutOfRoom || (CxPlatGetAllocFailDenominator() != 0).

This assert generally means that for some reason we thought we needed to call QuicSendWriteFrames to put some stuff in a packet, but when we ran through the logic, we ended up not doing anything. This usually indicates a bug in our logic. The complexity comes from the fact that we have related logic in two places:

  1. We have logic that tries to decide if we need to send anything in a particular packet type, in QuicSendFlush.
  2. In QuicSendWriteFrames we have the actual logic to write data to a packet.

Rarely, we have a bug that indicates these two are somehow out of sync. I'm wondering if somehow we need to send a PING frame for one thing, but then path probing kicks in (because there is a UDP port change) and we end up sending the PING in a different code path instead (where we might need to send it for both places?)....