diff --git a/proxy/http/remap/NextHopStrategyFactory.cc b/proxy/http/remap/NextHopStrategyFactory.cc index 6574ef52922..1bfc9ec04ef 100644 --- a/proxy/http/remap/NextHopStrategyFactory.cc +++ b/proxy/http/remap/NextHopStrategyFactory.cc @@ -229,6 +229,7 @@ NextHopStrategyFactory::loadConfigFile(const std::string fileName, std::stringst } } } + closedir(dir); } else { std::ifstream file(fileName); if (file.is_open()) { diff --git a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc index e3a57cbab92..91a168568f2 100644 --- a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc +++ b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc @@ -32,32 +32,32 @@ #include "HttpTransact.h" void -br(HttpRequestData *h, const char *os_hostname, sockaddr const *dest_ip) +br_destroy(HttpRequestData &h) { - HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); - h->hdr = new HTTPHdr(); - h->hdr->create(HTTP_TYPE_REQUEST, heap); - h->hostname_str = ats_strdup(os_hostname); - h->xact_start = time(nullptr); - ink_zero(h->src_ip); - ink_zero(h->dest_ip); - ats_ip_copy(&h->dest_ip.sa, dest_ip); - h->incoming_port = 80; - h->api_info = new HttpApiInfo(); + delete h.hdr; + delete h.api_info; + ats_free(h.hostname_str); } void -br_reinit(HttpRequestData *h) +build_request(HttpRequestData &h, const char *os_hostname) { - if (h->hdr) { - ats_free(h->hdr); - ats_free(h->hostname_str); - if (h->hdr) { - delete h->hdr; - } - if (h->api_info) { - delete h->api_info; - } + HdrHeap *heap = nullptr; + + if (h.hdr == nullptr) { + h.hdr = new HTTPHdr(); + h.hdr->create(HTTP_TYPE_REQUEST, heap); + h.xact_start = time(nullptr); + h.incoming_port = 80; + ink_zero(h.src_ip); + ink_zero(h.dest_ip); + } + if (h.hostname_str != nullptr) { + ats_free(h.hostname_str); + h.hostname_str = ats_strdup(os_hostname); + } + if (h.api_info == nullptr) { + h.api_info = new HttpApiInfo(); } } diff --git a/proxy/http/remap/unit-tests/nexthop_test_stubs.h b/proxy/http/remap/unit-tests/nexthop_test_stubs.h index e879bc2f938..bc05c5075b7 100644 --- a/proxy/http/remap/unit-tests/nexthop_test_stubs.h +++ b/proxy/http/remap/unit-tests/nexthop_test_stubs.h @@ -45,7 +45,11 @@ extern "C" { #define NH_Note(fmt, ...) PrintToStdErr("%s:%d:%s() " fmt "\n", __FILE__, __LINE__, __func__, ##__VA_ARGS__) #define NH_Warn(fmt, ...) PrintToStdErr("%s:%d:%s() " fmt "\n", __FILE__, __LINE__, __func__, ##__VA_ARGS__) +class HttpRequestData; + void PrintToStdErr(const char *fmt, ...); +void br_destroy(HttpRequestData &h); +void build_request(HttpRequestData &h, const char *os_hostname); #ifdef __cplusplus } diff --git a/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc b/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc index 08f11794383..f831253064e 100644 --- a/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc +++ b/proxy/http/remap/unit-tests/test_NextHopConsistentHash.cc @@ -41,7 +41,7 @@ extern int cmd_disable_pfreelist; SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", "[NextHopConsistentHash]") { - // We need this to build a HdrHeap object in br(); + // We need this to build a HdrHeap object in build_request(); // No thread setup, forbid use of thread local allocators. cmd_disable_pfreelist = true; // Get all of the HTTP WKS items populated. @@ -92,7 +92,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", REQUIRE(strategy != nullptr); // first request. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(10001, result, request, fail_threshold, retry_time); @@ -106,7 +106,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // second request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10002, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -117,7 +117,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // third request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10003, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -128,7 +128,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // fourth request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10004, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -139,7 +139,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // fifth request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10005, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -149,7 +149,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", strategy->markNextHopDown(10005, result, 1, fail_threshold); // sixth request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10006, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -159,7 +159,7 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", strategy->markNextHopDown(10006, result, 1, fail_threshold); // seventh request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10007, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_DIRECT); @@ -170,18 +170,20 @@ SCENARIO("Testing NextHopConsistentHash class, using policy 'consistent_hash'", // eighth request - reusing the ParentResult from the last request // simulating a failure triggers a search for another parent, not firstcall. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); strategy->findNextHop(10008, result, request, fail_threshold, retry_time, now); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result.hostname, "q2.bar.com") == 0); } + // free up request resources. + br_destroy(request); } } } SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'consistent_hash'", "[NextHopConsistentHash]") { - // We need this to build a HdrHeap object in br(); + // We need this to build a HdrHeap object in build_request(); // No thread setup, forbid use of thread local allocators. cmd_disable_pfreelist = true; // Get all of the HTTP WKS items populated. @@ -224,7 +226,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co REQUIRE(strategy != nullptr); // first request. - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(20001, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -233,7 +235,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co // mark down p1.foo.com strategy->markNextHopDown(20001, result, 1, fail_threshold); // second request - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(20002, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -243,7 +245,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co strategy->markNextHopDown(20002, result, 1, fail_threshold); // third request - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(20003, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -253,7 +255,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co strategy->markNextHopDown(20003, result, 1, fail_threshold); // fourth request - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(20004, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -263,7 +265,7 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co strategy->markNextHopDown(20004, result, 1, fail_threshold); // fifth request - br(&request, "rabbit.net/asset1"); + build_request(request, "rabbit.net/asset1"); result.reset(); strategy->findNextHop(20005, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -271,19 +273,21 @@ SCENARIO("Testing NextHopConsistentHash class (all firstcalls), using policy 'co // sixth request - wait and p1 should now become available time_t now = time(nullptr) + 5; - br(&request, "rabbit.net"); + build_request(request, "rabbit.net"); result.reset(); strategy->findNextHop(20006, result, request, fail_threshold, retry_time, now); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result.hostname, "p1.foo.com") == 0); } + // free up request resources. + br_destroy(request); } } } SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy 'consistent_hash'", "[NextHopConsistentHash]") { - // We need this to build a HdrHeap object in br(); + // We need this to build a HdrHeap object in build_request(); // No thread setup, forbid use of thread local allocators. cmd_disable_pfreelist = true; // Get all of the HTTP WKS items populated. @@ -321,7 +325,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy REQUIRE(strategy != nullptr); // first request. - br(&request, "bunny.net/asset1"); + build_request(request, "bunny.net/asset1"); result.reset(); strategy->findNextHop(30001, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -331,7 +335,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy strategy->markNextHopDown(30001, result, 1, fail_threshold); // second request - br(&request, "bunny.net.net/asset1"); + build_request(request, "bunny.net.net/asset1"); strategy->findNextHop(30002, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result.hostname, "c3.bar.com") == 0); @@ -340,7 +344,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy strategy->markNextHopDown(30002, result, 1, fail_threshold); // third request - br(&request, "bunny.net/asset2"); + build_request(request, "bunny.net/asset2"); result.reset(); strategy->findNextHop(30003, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -349,7 +353,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // just mark it down and retry request strategy->markNextHopDown(30003, result, 1, fail_threshold); // fourth request - br(&request, "bunny.net/asset2"); + build_request(request, "bunny.net/asset2"); strategy->findNextHop(30004, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result.hostname, "c1.foo.com") == 0); @@ -357,7 +361,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // mark it down strategy->markNextHopDown(30004, result, 1, fail_threshold); // fifth request - new request - br(&request, "bunny.net/asset3"); + build_request(request, "bunny.net/asset3"); result.reset(); strategy->findNextHop(30005, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -366,7 +370,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // mark it down and retry strategy->markNextHopDown(30005, result, 1, fail_threshold); // sixth request - br(&request, "bunny.net/asset3"); + build_request(request, "bunny.net/asset3"); result.reset(); strategy->findNextHop(30006, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); @@ -375,7 +379,7 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // mark it down strategy->markNextHopDown(30006, result, 1, fail_threshold); // seventh request - new request with all hosts down and go_direct is false. - br(&request, "bunny.net/asset4"); + build_request(request, "bunny.net/asset4"); result.reset(); strategy->findNextHop(30007, result, request, fail_threshold, retry_time); CHECK(result.result == ParentResultType::PARENT_FAIL); @@ -383,12 +387,14 @@ SCENARIO("Testing NextHopConsistentHash class (alternating rings), using policy // eighth request - retry after waiting for the retry window to expire. time_t now = time(nullptr) + 5; - br(&request, "bunny.net/asset4"); + build_request(request, "bunny.net/asset4"); result.reset(); strategy->findNextHop(30008, result, request, fail_threshold, retry_time, now); CHECK(result.result == ParentResultType::PARENT_SPECIFIED); CHECK(strcmp(result.hostname, "c2.foo.com") == 0); } + // free up request resources. + br_destroy(request); } } }