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

Memory leakage when rejecting HTTP2 server push stream #1229

Closed
zelinchen opened this Issue Jan 26, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@zelinchen

zelinchen commented Jan 26, 2017

I did this

I've copied the code form https://curl.haxx.se/libcurl/c/http2-serverpush.html. And just return CURL_PUSH_DENY in server_push_callback() to test if can reject server push steam. All is fine except that valgrind complains about memory leakage.

I expected the following

No memory leakage even rejecting server push stream.

curl/libcurl version

libcurl: curl-7_52_1
nghttp2: v1.18.1

operating system

Fedora 19, x86_64

I think better to destroy the following 2 pointers when failure or rejecting steam.

diff --git a/lib/http2.c b/lib/http2.c
index 4cc17ba..fa7bdb6 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -418,6 +418,9 @@ static int push_promise(struct Curl_easy *data,

     if(rv) {
       /* denied, kill off the new handle again */
+      Curl_add_buffer_free(
+        ((struct HTTP *)(newhandle->req.protop))->header_recvbuf);
+      free(newhandle->req.protop);
       (void)Curl_close(newhandle);
       goto fail;
     }
@@ -432,6 +435,9 @@ static int push_promise(struct Curl_easy *data,
     rc = Curl_multi_add_perform(data->multi, newhandle, conn);
     if(rc) {
       infof(data, "failed to add handle to multi\n");
+      Curl_add_buffer_free(
+        ((struct HTTP *)(newhandle->req.protop))->header_recvbuf);
+      free(newhandle->req.protop);
       Curl_close(newhandle);
       rv = 1;
       goto fail;

@bagder bagder self-assigned this Feb 12, 2017

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Feb 12, 2017

Member

Reproduced.

Member

bagder commented Feb 12, 2017

Reproduced.

@bagder bagder closed this in bde1e2e Feb 13, 2017

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Feb 13, 2017

Member

Thanks a lot. As you can see I opted to solving the problem slightly different with the hope that this should make it less likely to happen again.

Member

bagder commented Feb 13, 2017

Thanks a lot. As you can see I opted to solving the problem slightly different with the hope that this should make it less likely to happen again.

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

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