-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Conversation
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`.
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 |
Not all of the quic tests will pass on all platforms yet |
@nodejs/quic ... fast track? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
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 <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Landed in 133a97f |
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), orvcbuild test
(Windows) passes