Skip to content

Commit aa7d3d8

Browse files
committed
Track created curl_slist structs by option so they can be updated in situ.
At present, when curl_setopt() is called with an option that requires the creation of a curl_slist, we simply push the new curl_slist onto a list to be freed when the curl handle is freed. This avoids a memory leak, but means that repeated calls to curl_setopt() on the same handle with the same option wastes previously allocated memory on curl_slist structs that will no longer be read. This commit changes the zend_llist that was previously used to track the lists to a HashTable keyed by the option number, which means that we can simply update the hash table each time curl_setopt() is called. Fixes bug #65458 (curl memory leak).
1 parent a486250 commit aa7d3d8

File tree

4 files changed

+38
-6
lines changed

4 files changed

+38
-6
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ PHP NEWS
1919
. Fixed bug #61268 (--enable-dtrace leads make to clobber
2020
Zend/zend_dtrace.d) (Chris Jones)
2121

22+
- cURL:
23+
. Fixed bug #65458 (curl memory leak). (Adam)
24+
2225
- Openssl:
2326
. Fixed bug #64802 (openssl_x509_parse fails to parse subject properly in
2427
some cases). (Mark Jones)

ext/curl/interface.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,9 +1373,9 @@ static void curl_free_post(void **post)
13731373

13741374
/* {{{ curl_free_slist
13751375
*/
1376-
static void curl_free_slist(void **slist)
1376+
static void curl_free_slist(void *slist)
13771377
{
1378-
curl_slist_free_all((struct curl_slist *) *slist);
1378+
curl_slist_free_all(*((struct curl_slist **) slist));
13791379
}
13801380
/* }}} */
13811381

@@ -1443,8 +1443,10 @@ static void alloc_curl_handle(php_curl **ch)
14431443
(*ch)->handlers->read->stream = NULL;
14441444

14451445
zend_llist_init(&(*ch)->to_free->str, sizeof(char *), (llist_dtor_func_t) curl_free_string, 0);
1446-
zend_llist_init(&(*ch)->to_free->slist, sizeof(struct curl_slist), (llist_dtor_func_t) curl_free_slist, 0);
14471446
zend_llist_init(&(*ch)->to_free->post, sizeof(struct HttpPost), (llist_dtor_func_t) curl_free_post, 0);
1447+
1448+
(*ch)->to_free->slist = emalloc(sizeof(HashTable));
1449+
zend_hash_init((*ch)->to_free->slist, 4, NULL, curl_free_slist, 0);
14481450
}
14491451
/* }}} */
14501452

@@ -1675,6 +1677,7 @@ PHP_FUNCTION(curl_copy_handle)
16751677
curl_easy_setopt(dupch->cp, CURLOPT_WRITEHEADER, (void *) dupch);
16761678
curl_easy_setopt(dupch->cp, CURLOPT_PROGRESSDATA, (void *) dupch);
16771679

1680+
efree(dupch->to_free->slist);
16781681
efree(dupch->to_free);
16791682
dupch->to_free = ch->to_free;
16801683

@@ -2184,7 +2187,7 @@ static int _php_curl_setopt(php_curl *ch, long option, zval **zvalue, zval *retu
21842187
return 1;
21852188
}
21862189
}
2187-
zend_llist_add_element(&ch->to_free->slist, &slist);
2190+
zend_hash_index_update(ch->to_free->slist, (ulong) option, &slist, sizeof(struct curl_slist *), NULL);
21882191

21892192
error = curl_easy_setopt(ch->cp, option, slist);
21902193

@@ -2680,8 +2683,9 @@ static void _php_curl_close_ex(php_curl *ch TSRMLS_DC)
26802683
/* cURL destructors should be invoked only by last curl handle */
26812684
if (Z_REFCOUNT_P(ch->clone) <= 1) {
26822685
zend_llist_clean(&ch->to_free->str);
2683-
zend_llist_clean(&ch->to_free->slist);
26842686
zend_llist_clean(&ch->to_free->post);
2687+
zend_hash_destroy(ch->to_free->slist);
2688+
efree(ch->to_free->slist);
26852689
efree(ch->to_free);
26862690
FREE_ZVAL(ch->clone);
26872691
} else {

ext/curl/php_curl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ struct _php_curl_send_headers {
126126
struct _php_curl_free {
127127
zend_llist str;
128128
zend_llist post;
129-
zend_llist slist;
129+
HashTable *slist;
130130
};
131131

132132
typedef struct {

ext/curl/tests/bug65458.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Bug #65458 (curl memory leak)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('curl')) exit("skip curl extension not loaded");
6+
?>
7+
--FILE--
8+
<?php
9+
$ch = curl_init();
10+
$init = memory_get_usage();
11+
for ($i = 0; $i < 10000; $i++) {
12+
curl_setopt($ch, CURLOPT_HTTPHEADER, [ "SOAPAction: getItems" ]);
13+
}
14+
15+
$preclose = memory_get_usage();
16+
curl_close($ch);
17+
18+
// This is a slightly tricky heuristic, but basically, we want to ensure
19+
// $preclose - $init has a delta in the order of bytes, not megabytes. Given
20+
// the number of iterations in the loop, if we're wasting memory here, we
21+
// should have megs and megs of extra allocations.
22+
var_dump(($preclose - $init) < 10000);
23+
?>
24+
--EXPECT--
25+
bool(true)

0 commit comments

Comments
 (0)