Skip to content

quic: fix stall datagrams, if no pending streams#64303

Open
martenrichter wants to merge 2 commits into
nodejs:mainfrom
martenrichter:fixdatagramstall
Open

quic: fix stall datagrams, if no pending streams#64303
martenrichter wants to merge 2 commits into
nodejs:mainfrom
martenrichter:fixdatagramstall

Conversation

@martenrichter

Copy link
Copy Markdown
Contributor

If you are only sending datagrams and so streams,
your datagrams could stall.
There were two reasons:
i. A SendPendingDataScope in SendDatagrams was
missing.
ii. SendPendingData did not attempt to send
datagrams, if there was no stream data.

Also two tests needed changes.

If you are only sending datagrams and so streams,
your datagrams could stall.
There were two reasons:
i. A SendPendingDataScope in SendDatagrams was
missing.
ii. SendPendingData did not attempt to send
datagrams, if there were no stream data.

Signed-off-by: Marten Richter <marten.richter@freenet.de>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Jul 5, 2026
@martenrichter

Copy link
Copy Markdown
Contributor Author

@jasnell @pimterry

This may conflict with the PR of #64234 .
So a rebase might be necessary.

The changes of the tests are a bit of a cover up. So suggestions are welcome.

The underlying problem is that ngtcp2 does not indicate that instead of a datagram, another packet is sent. (In this case, it is the acknowledgment of the reestablished connection; I have debugged it, and it will run several times to wait for some time to pass.). So also these waiting counts as failure.
So in fact it will require a change in upstream. I have also checked if waiting for path validation on client side is enough.
Or another callback to indicate, that the connection is fully working again, what we do not have.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants