From 51ef1a207d95d12501a39383d1fff5ada082a2ba Mon Sep 17 00:00:00 2001 From: Alexei Pastuchov Date: Fri, 17 Jul 2026 22:26:44 +0200 Subject: [PATCH] Release get_free_port() reservations not tied to a Server's primary port Since #472, get_free_port() leaves its reservation socket bound until release_port() is called, and Server::start() only releases its own _port. Several call sites reserve a port for something other than a Server's primary port and never released it, leaking the reservation socket for the rest of the test process: - Context (tests/context.h) and cycle_context_st (tests/cycle.cc) and round_robin.cc's Context overwrite their stored port in reset() without releasing the old one first, and never release the final port on destruction. - httpd_test.cc reserves an http_port passed only as --http-port= to a server whose own primary port is default_port(). - gearmand.cc's long_keepalive_start_TEST and config_file_SIMPLE_TEST reserve a port for --check-args validation runs that never bind it. - unittest.cc's get_free_port_TEST exercises get_free_port() directly without ever releasing what it reserves. default_port()'s single process-wide reservation is intentionally left untouched, since callers rely on it staying valid for the process's whole lifetime. Fixes #474 Signed-off-by: Alexei Pastuchov --- libtest/unittest.cc | 17 +++++++++++++++-- tests/context.h | 2 ++ tests/cycle.cc | 6 ++++++ tests/gearmand.cc | 8 ++++++++ tests/httpd_test.cc | 8 +++++++- tests/round_robin.cc | 4 +++- 6 files changed, 41 insertions(+), 4 deletions(-) diff --git a/libtest/unittest.cc b/libtest/unittest.cc index 6981e3827..80ddadba6 100644 --- a/libtest/unittest.cc +++ b/libtest/unittest.cc @@ -921,8 +921,21 @@ static test_return_t get_free_port_TEST(void *) { in_port_t ret_port; ASSERT_TRUE((ret_port= get_free_port())); - ASSERT_TRUE(get_free_port() != default_port()); - ASSERT_TRUE(get_free_port() != get_free_port()); + + in_port_t second_port= get_free_port(); + ASSERT_TRUE(second_port != default_port()); + + in_port_t third_port= get_free_port(); + in_port_t fourth_port= get_free_port(); + ASSERT_TRUE(third_port != fourth_port); + + // These are throwaway reservations never tied to a Server, so release them + // ourselves; default_port()'s own reservation is intentionally left alone + // since it stays valid for the lifetime of the process. + release_port(ret_port); + release_port(second_port); + release_port(third_port); + release_port(fourth_port); return TEST_SUCCESS; } diff --git a/tests/context.h b/tests/context.h index ca839cb43..22f208836 100644 --- a/tests/context.h +++ b/tests/context.h @@ -66,6 +66,7 @@ class Context { extra_clear(); _servers.clear(); + libtest::release_port(_port); } const char *worker_function_name() const @@ -123,6 +124,7 @@ class Context _worker= NULL; extra_clear(); + libtest::release_port(_port); _port= get_free_port(); } }; diff --git a/tests/cycle.cc b/tests/cycle.cc index a420c0361..2f8b84e79 100644 --- a/tests/cycle.cc +++ b/tests/cycle.cc @@ -56,9 +56,15 @@ struct cycle_context_st reset(); } + ~cycle_context_st() + { + libtest::release_port(port); + } + void reset() { servers.clear(); + libtest::release_port(port); port= get_free_port(); } diff --git a/tests/gearmand.cc b/tests/gearmand.cc index 5701c7ab3..a25945af4 100644 --- a/tests/gearmand.cc +++ b/tests/gearmand.cc @@ -191,6 +191,10 @@ static test_return_t long_keepalive_start_TEST(void *) char port_str[1024]; ASSERT_TRUE(snprintf(port_str, sizeof(port_str), "--port=%d", int32_t(port)) > 0); + // --check-args only validates arguments; gearmand never binds this port, + // so there's nothing for anyone else to release it later. + libtest::release_port(port); + const char *args[]= { port_str, "--check-args", @@ -550,6 +554,10 @@ static test_return_t config_file_SIMPLE_TEST(void *) char port_str[1024]; ASSERT_TRUE(snprintf(port_str, sizeof(port_str), "%d", int32_t(port)) > 0); + // --check-args only validates the config file; gearmand never binds this + // port, so there's nothing for anyone else to release it later. + libtest::release_port(port); + std::string config_file= "etc/gearmand.conf"; { char current_directory_buffer[1024]; diff --git a/tests/httpd_test.cc b/tests/httpd_test.cc index 0c2c85f1c..23a6c666f 100644 --- a/tests/httpd_test.cc +++ b/tests/httpd_test.cc @@ -154,7 +154,13 @@ static void *world_create(server_startup_st& servers, test_return_t& error) length= snprintf(buffer, sizeof(buffer), "--http-port=%d", int(http_port)); fatal_assert(length > 0 and sizeof(length) < sizeof(buffer)); const char *argv[]= { "--protocol=http", buffer, 0 }; - if (server_startup(servers, "gearmand", libtest::default_port(), argv) == false) + bool started= server_startup(servers, "gearmand", libtest::default_port(), argv); + + // http_port is only ever used as the --http-port= value above; it's never + // the primary port of a Server object, so nothing else will release it. + libtest::release_port(http_port); + + if (started == false) { error= TEST_SKIPPED; return NULL; diff --git a/tests/round_robin.cc b/tests/round_robin.cc index 8b8e463bf..38f37faf0 100644 --- a/tests/round_robin.cc +++ b/tests/round_robin.cc @@ -59,12 +59,14 @@ struct Context ~Context() { - reset(); + servers.clear(); + libtest::release_port(_port); } void reset() { servers.clear(); + libtest::release_port(_port); _port= libtest::get_free_port(); _retries= 0; }