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: return 0 from SSL_CTX_sess_set_new_cb callback #33931

Closed

Conversation

addaleax
Copy link
Member

The callback passed to SSL_CTX_sess_set_new_cb() should return 1
if it takes ownership (i.e. holds a reference that is later freed
through SSL_SESSION_free()) of the passed session, and 0
otherwise.

Since we don’t take ownership of the session and instead only save
a serialized version of it, return 0 instead of 1. This closes
a memory leak reported when running
sequential/test-quic-preferred-address-ipv6.

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

The callback passed to `SSL_CTX_sess_set_new_cb()` should return 1
if it takes ownership (i.e. holds a reference that is later freed
through `SSL_SESSION_free()`) of the passed session, and 0
otherwise.

Since we don’t take ownership of the session and instead only save
a serialized version of it, return 0 instead of 1. This closes
a memory leak reported when running
`sequential/test-quic-preferred-address-ipv6`.
@addaleax addaleax added the quic Issues and PRs related to the QUIC implementation / HTTP/3. label Jun 17, 2020
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 17, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 17, 2020

@addaleax addaleax requested a review from jasnell June 17, 2020 18:20
@jasnell jasnell added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. experimental Issues and PRs related to experimental features. labels Jun 19, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

jasnell pushed a commit that referenced this pull request Jun 22, 2020
The callback passed to `SSL_CTX_sess_set_new_cb()` should return 1
if it takes ownership (i.e. holds a reference that is later freed
through `SSL_SESSION_free()`) of the passed session, and 0
otherwise.

Since we don’t take ownership of the session and instead only save
a serialized version of it, return 0 instead of 1. This closes
a memory leak reported when running
`sequential/test-quic-preferred-address-ipv6`.

PR-URL: #33931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@jasnell
Copy link
Member

jasnell commented Jun 22, 2020

Landed in 0e7c8bd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. experimental Issues and PRs related to experimental features. quic Issues and PRs related to the QUIC implementation / HTTP/3.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants