From 409f2a041f752ef635354e1dc7b2759fcd7088d6 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sun, 18 Nov 2012 16:17:37 +0100 Subject: [PATCH] fixed memory leak: CURLOPT_RESOLVE with multi interface DNS cache entries populated with CURLOPT_RESOLVE were not properly freed again when done using the multi interface. Test case 1502 added to verify. Bug: http://curl.haxx.se/bug/view.cgi?id=3575448 Reported by: Alex Gruz --- lib/hostip.c | 8 ++- lib/hostip.h | 10 ++- lib/multi.c | 13 ++-- tests/data/Makefile.am | 2 +- tests/data/test1502 | 58 ++++++++++++++++ tests/libtest/Makefile.inc | 4 +- tests/libtest/lib1502.c | 138 +++++++++++++++++++++++++++++++++++++ 7 files changed, 219 insertions(+), 14 deletions(-) create mode 100644 tests/data/test1502 create mode 100644 tests/libtest/lib1502.c diff --git a/lib/hostip.c b/lib/hostip.c index 503ba483f..ef14ce863 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2011, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -740,14 +740,18 @@ static int hostcache_inuse(void *data, void *hc) return 1; /* free all entries */ } -void Curl_hostcache_destroy(struct SessionHandle *data) +void Curl_hostcache_clean(struct SessionHandle *data) { /* Entries added to the hostcache with the CURLOPT_RESOLVE function are * still present in the cache with the inuse counter set to 1. Detect them * and cleanup! */ Curl_hash_clean_with_criterium(data->dns.hostcache, data, hostcache_inuse); +} +void Curl_hostcache_destroy(struct SessionHandle *data) +{ + Curl_hostcache_clean(data); Curl_hash_destroy(data->dns.hostcache); data->dns.hostcachetype = HCACHE_NONE; data->dns.hostcache = NULL; diff --git a/lib/hostip.h b/lib/hostip.h index 12fb80f54..de71f54f4 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2011, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -200,11 +200,19 @@ extern sigjmp_buf curl_jmpenv; */ CURLcode Curl_set_dns_servers(struct SessionHandle *data, char *servers); +/* + * Clean off entries from the cache + */ +void Curl_hostcache_clean(struct SessionHandle *data); + /* * Destroy the hostcache of this handle. */ void Curl_hostcache_destroy(struct SessionHandle *data); +/* + * Populate the cache with specified entries from CURLOPT_RESOLVE. + */ CURLcode Curl_loadhostpairs(struct SessionHandle *data); #endif /* HEADER_CURL_HOSTIP_H */ diff --git a/lib/multi.c b/lib/multi.c index 938846769..b4e01bd93 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1789,12 +1789,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } WHILE_FALSE; /* just to break out from! */ if(CURLM_STATE_COMPLETED == easy->state) { - if(data->dns.hostcachetype == HCACHE_MULTI) { - /* clear out the usage of the shared DNS cache */ - data->dns.hostcache = NULL; - data->dns.hostcachetype = HCACHE_NONE; - } - /* now fill in the Curl_message with this info */ msg = &easy->msg; @@ -1911,9 +1905,6 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) cl= n; } - Curl_hash_destroy(multi->hostcache); - multi->hostcache = NULL; - Curl_hash_destroy(multi->sockhash); multi->sockhash = NULL; @@ -1930,6 +1921,7 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) nexteasy=easy->next; if(easy->easy_handle->dns.hostcachetype == HCACHE_MULTI) { /* clear out the usage of the shared DNS cache */ + Curl_hostcache_clean(easy->easy_handle); easy->easy_handle->dns.hostcache = NULL; easy->easy_handle->dns.hostcachetype = HCACHE_NONE; } @@ -1943,6 +1935,9 @@ CURLMcode curl_multi_cleanup(CURLM *multi_handle) easy = nexteasy; } + Curl_hash_destroy(multi->hostcache); + multi->hostcache = NULL; + free(multi); return CURLM_OK; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 9210404a5..0528a2567 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -93,7 +93,7 @@ test1379 test1380 test1381 test1382 test1383 test1384 test1385 test1386 \ test1387 test1388 test1389 test1390 test1391 test1392 test1393 \ test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \ test1408 test1409 test1410 test1411 test1412 test1413 \ -test1500 test1501 \ +test1500 test1501 test1502 \ test2000 test2001 test2002 test2003 test2004 test2005 test2006 test2007 \ test2008 test2009 test2010 test2011 test2012 test2013 test2014 test2015 \ test2016 test2017 test2018 test2019 test2020 test2021 test2022 \ diff --git a/tests/data/test1502 b/tests/data/test1502 new file mode 100644 index 000000000..3e3b36552 --- /dev/null +++ b/tests/data/test1502 @@ -0,0 +1,58 @@ + + + +HTTP +HTTP GET +multi +CURLOPT_RESOLVE + + + + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 6 +Connection: close +Content-Type: text/html +Funny-head: yesyes + +-foo- + + + +# +# Client-side + + +http + + +lib1502 + + +HTTP multi with CURLOPT_RESOLVE + + +http://google.com:%HTTPPORT/1502 %HTTPPORT %HOSTIP + + + +# +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /1502 HTTP/1.1 +Host: google.com:%HTTPPORT +Accept: */* + + + + diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 3cbc45aad..b568e190e 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -20,7 +20,7 @@ noinst_PROGRAMS = chkhostname \ lib556 lib539 lib557 lib560 lib562 lib564 lib565 lib566 lib567 lib568 \ lib569 lib570 lib571 lib572 lib573 lib582 lib583 lib585 lib586 lib587 \ lib590 lib591 lib597 lib598 lib599 libauthretry libntlmconnect \ - lib1500 lib1501 + lib1500 lib1501 lib1502 chkhostname_SOURCES = chkhostname.c $(top_srcdir)/lib/curl_gethostname.c chkhostname_LDADD = @CURL_NETWORK_LIBS@ @@ -191,6 +191,8 @@ lib1500_SOURCES = lib1500.c $(SUPPORTFILES) $(TESTUTIL) lib1501_SOURCES = lib1501.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib1502_SOURCES = lib1502.c $(SUPPORTFILES) + libauthretry_SOURCES = libauthretry.c $(SUPPORTFILES) libntlmconnect_SOURCES = libntlmconnect.c $(SUPPORTFILES) $(TESTUTIL) diff --git a/tests/libtest/lib1502.c b/tests/libtest/lib1502.c new file mode 100644 index 000000000..ec1058826 --- /dev/null +++ b/tests/libtest/lib1502.c @@ -0,0 +1,138 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at http://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ +/* + * Test case converted from bug report #3575448, identifying a memory leak in + * the CURLOPT_RESOLVE handling with the multi interface. + */ + +#include "test.h" + +#include +#include + +/* somewhat unix-specific */ +#include +#include + +int test(char *URL) +{ + CURL *ehandle; + CURLM *multi_handle; + char redirect[160]; + CURLcode result = CURLE_NOT_BUILT_IN; + int still_running; /* keep number of running handles */ + + CURLMsg *msg; /* for picking up messages with the transfer status */ + int msgs_left; /* how many messages are left */ + + /* DNS cache injection */ + struct curl_slist *dns_cache_list; + + sprintf(redirect, "google.com:%s:%s", libtest_arg2, libtest_arg3); + + fprintf(stderr, "Redirect: %s\n", redirect); + + dns_cache_list = curl_slist_append(NULL, redirect); + + /* Allocate one CURL handle per transfer */ + ehandle = curl_easy_init(); + + /* set the options (I left out a few, you'll get the point anyway) */ + curl_easy_setopt(ehandle, CURLOPT_URL, URL); + curl_easy_setopt(ehandle, CURLOPT_HEADER, 1L); + + curl_easy_setopt(ehandle, CURLOPT_RESOLVE, dns_cache_list); + + /* init a multi stack */ + multi_handle = curl_multi_init(); + + /* add the individual transfers */ + curl_multi_add_handle(multi_handle, ehandle); + + /* we start some action by calling perform right away */ + curl_multi_perform(multi_handle, &still_running); + + do { + struct timeval timeout; + int rc; /* select() return code */ + + fd_set fdread; + fd_set fdwrite; + fd_set fdexcep; + int maxfd = -1; + + long curl_timeo = -1; + + FD_ZERO(&fdread); + FD_ZERO(&fdwrite); + FD_ZERO(&fdexcep); + + /* set a suitable timeout to play around with */ + timeout.tv_sec = 1; + timeout.tv_usec = 0; + + curl_multi_timeout(multi_handle, &curl_timeo); + if(curl_timeo >= 0) { + timeout.tv_sec = curl_timeo / 1000; + if(timeout.tv_sec > 1) + timeout.tv_sec = 1; + else + timeout.tv_usec = (curl_timeo % 1000) * 1000; + } + + /* get file descriptors from the transfers */ + curl_multi_fdset(multi_handle, &fdread, &fdwrite, &fdexcep, &maxfd); + + /* In a real-world program you OF COURSE check the return code of the + function calls. On success, the value of maxfd is guaranteed to be + greater or equal than -1. We call select(maxfd + 1, ...), specially in + case of (maxfd == -1), we call select(0, ...), which is basically equal + to sleep. */ + + rc = select(maxfd+1, &fdread, &fdwrite, &fdexcep, &timeout); + + switch(rc) { + case -1: + /* select error */ + break; + case 0: /* timeout */ + default: /* action */ + curl_multi_perform(multi_handle, &still_running); + break; + } + } while(still_running); + + /* See how the transfers went */ + while ((msg = curl_multi_info_read(multi_handle, &msgs_left))) { + if (msg->msg == CURLMSG_DONE) + result = msg->data.result; + } + + curl_multi_cleanup(multi_handle); + + /* Free the CURL handles */ + curl_easy_cleanup(ehandle); + + curl_slist_free_all(dns_cache_list); + + return (int)result; +}