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

Jump to bottom

Memory leakage when rejecting HTTP2 server push stream #1229

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

Memory leakage when rejecting HTTP2 server push stream #1229

zelinchen opened this issue Jan 26, 2017 · 2 comments
Assignees
@bagder
Labels
HTTP/2 memory-leak

Comments

@zelinchen
Copy link

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
Copy link
Member

bagder commented Feb 12, 2017

Reproduced.

@bagder bagder closed this as completed in bde1e2e Feb 13, 2017
@bagder
Copy link
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.
Assignees

@bagder bagder

Labels
HTTP/2 memory-leak
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bagder @zelinchen