Skip to content

Commit 52a6e23

Browse files
authored
fix(agent): Fix nr_header_create_distributed_trace_map mem leak when hashmap is destroyed (#1017)
This function created a new hashmap, but didn's pass the string dtor in so any strdupped values were not being freed when the hashmap was destroyed. valgrind output from a multiverse run showed: ==220== by 0x6AA4680: nr_strdup (util_memory.c:156) ==220== by 0x6A83BD3: nr_header_create_distributed_trace_map (nr_header.c:60) ==220== by 0x6A45D28: nr_php_amqplib_retrieve_dt_headers (lib_php_amqplib.c:503) ==220== by 0x6A45D28: nr_rabbitmq_basic_get (lib_php_amqplib.c:742) ==220== by 0x6A74993: nr_zend_call_orig_execute_special (php_user_instrument.c:105) ==220== by 0x6A52EAA: nr_php_instrument_func_end (php_execute.c:2086) ==220== by 0x6A54D5B: nr_php_observer_fcall_end (php_execute.c:2188) ==220== by 0x71D7ED: zend_observer_fcall_end (in /usr/local/bin/php) ==220== by 0x6E79FF: execute_ex (in /usr/local/bin/php) ==220== by 0x6F0B42: zend_execute (in /usr/local/bin/php) ==220== by 0x67C06F: zend_execute_scripts (in /usr/local/bin/php) ==220== by 0x60FA3D: php_execute_script (in /usr/local/bin/php) after applying the fix, valgrind had no issues.
1 parent b0184fd commit 52a6e23

File tree

2 files changed

+72
-1
lines changed

2 files changed

+72
-1
lines changed

axiom/nr_header.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ nr_hashmap_t* nr_header_create_distributed_trace_map(const char* nr_header,
4848
return NULL;
4949
}
5050

51-
header_map = nr_hashmap_create(NULL);
51+
header_map = nr_hashmap_create((nr_hashmap_dtor_func_t)nr_hashmap_dtor_str);
52+
5253
if (nr_header) {
5354
nr_hashmap_set(header_map, NR_PSTR(NEWRELIC), nr_strdup(nr_header));
5455
}

axiom/tests/test_header.c

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,6 +1702,75 @@ static void test_account_id_from_cross_process_id(void) {
17021702
nr_header_account_id_from_cross_process_id("10#10"));
17031703
}
17041704

1705+
static void test_nr_header_create_distributed_trace_map(void) {
1706+
nr_hashmap_t* header_map = NULL;
1707+
char* tracestate = "tracestate";
1708+
char* traceparent = "traceparent";
1709+
char* dt_payload = "newrelic";
1710+
1711+
header_map = nr_header_create_distributed_trace_map(NULL, NULL, NULL);
1712+
tlib_pass_if_null(
1713+
"NULL payload and NULL traceparent should return NULL header map",
1714+
header_map);
1715+
1716+
header_map = nr_header_create_distributed_trace_map(NULL, NULL, tracestate);
1717+
tlib_pass_if_null(
1718+
"NULL payload and NULL traceparent should return NULL header map",
1719+
header_map);
1720+
1721+
header_map = nr_header_create_distributed_trace_map(dt_payload, NULL, NULL);
1722+
tlib_pass_if_not_null("if valid dt_payload should return a header map",
1723+
header_map);
1724+
tlib_pass_if_size_t_equal(
1725+
"1 header passed in so should expect headers hashmap size of 1", 1,
1726+
nr_hashmap_count(header_map));
1727+
nr_hashmap_destroy(&header_map);
1728+
1729+
header_map
1730+
= nr_header_create_distributed_trace_map(dt_payload, traceparent, NULL);
1731+
tlib_pass_if_not_null("if valid dt_payload should return a header map",
1732+
header_map);
1733+
tlib_pass_if_size_t_equal(
1734+
"2 headers passed in so should expect headers hashmap size of 2", 2,
1735+
nr_hashmap_count(header_map));
1736+
nr_hashmap_destroy(&header_map);
1737+
1738+
header_map
1739+
= nr_header_create_distributed_trace_map(dt_payload, NULL, tracestate);
1740+
tlib_pass_if_not_null("if valid dt_payload should return a header map",
1741+
header_map);
1742+
tlib_pass_if_size_t_equal(
1743+
"2 headers passed in so should expect headers hashmap size of 2", 2,
1744+
nr_hashmap_count(header_map));
1745+
nr_hashmap_destroy(&header_map);
1746+
1747+
header_map = nr_header_create_distributed_trace_map(dt_payload, traceparent,
1748+
tracestate);
1749+
tlib_pass_if_not_null("if valid dt_payload should return a header map",
1750+
header_map);
1751+
tlib_pass_if_size_t_equal(
1752+
"3 headers passed in so should expect headers hashmap size of 3", 3,
1753+
nr_hashmap_count(header_map));
1754+
nr_hashmap_destroy(&header_map);
1755+
1756+
header_map
1757+
= nr_header_create_distributed_trace_map(NULL, traceparent, tracestate);
1758+
tlib_pass_if_not_null("if valid traceparent should return a header map",
1759+
header_map);
1760+
tlib_pass_if_size_t_equal(
1761+
"Two headers passed in so should expect headers hashmap size of 2", 2,
1762+
nr_hashmap_count(header_map));
1763+
nr_hashmap_destroy(&header_map);
1764+
1765+
header_map = nr_header_create_distributed_trace_map(NULL, traceparent, NULL);
1766+
tlib_pass_if_not_null("if valid traceparent should return a header map",
1767+
header_map);
1768+
tlib_pass_if_size_t_equal(
1769+
"1 header passed in so should expect headers hashmap size of 1", 1,
1770+
nr_hashmap_count(header_map));
1771+
nr_hashmap_destroy(&header_map);
1772+
}
1773+
17051774
tlib_parallel_info_t parallel_info = {.suggested_nthreads = 2, .state_size = 0};
17061775

17071776
void test_main(void* p NRUNUSED) {
@@ -1721,4 +1790,5 @@ void test_main(void* p NRUNUSED) {
17211790
test_set_cat_txn();
17221791
test_set_synthetics_txn();
17231792
test_account_id_from_cross_process_id();
1793+
test_nr_header_create_distributed_trace_map();
17241794
}

0 commit comments

Comments
 (0)