Sitelet https://web.archive.org/web/20220423162017/https://github.com/nodejs/node/pull/33917
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.

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

quic: always copy stateless reset token #33917

Closed
wants to merge 1 commit into from

Conversation

Copy link
Member

@addaleax addaleax commented Jun 16, 2020

Take ownership of the token value, since the memory for it is allocated
anyway and the buffer size is just 16, i.e. copyable very cheaply.

This makes valgrind stop complaining about a use-after-free error
when running sequential/test-quic-preferred-address-ipv6.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Take ownership of the token value, since the memory for it is allocated
anyway and the buffer size is just 16, i.e. copyable very cheaply.

This makes valgrind stop complaining about a use-after-free error
when running `sequential/test-quic-preferred-address-ipv6`.
@nodejs-github-bot nodejs-github-bot added the c++ label Jun 16, 2020
@addaleax addaleax added the quic label Jun 16, 2020
@jasnell
Copy link
Member

@jasnell jasnell commented Jun 17, 2020 •

CI: https://ci.nodejs.org/job/node-test-commit/39060/

test-quic-preferred-address-ipv6 failed on multiple systems in #33912, it's possible this fixes it, but I also think we need to skip ipv6 on some of the systems -- #33919

@jasnell
Copy link
Member

@jasnell jasnell commented Jun 17, 2020

Not all of the quic tests will pass on all platforms yet

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jun 17, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jun 17, 2020

@jasnell jasnell added the fast-track label Jun 17, 2020
@jasnell
Copy link
Member

@jasnell jasnell commented Jun 17, 2020

@nodejs/quic ... fast track?

Copy link
Member

@mcollina mcollina left a comment

lgtm

jasnell pushed a commit that referenced this issue Jun 17, 2020
Take ownership of the token value, since the memory for it is allocated
anyway and the buffer size is just 16, i.e. copyable very cheaply.

This makes valgrind stop complaining about a use-after-free error
when running `sequential/test-quic-preferred-address-ipv6`.

PR-URL: #33917
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@jasnell
Copy link
Member

@jasnell jasnell commented Jun 17, 2020

Landed in 133a97f

@jasnell jasnell closed this Jun 17, 2020
@codebytere codebytere added the dont-land-on-v14.x label Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ dont-land-on-v14.x fast-track quic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants