Compare commits

..

2 Commits

Author SHA1 Message Date
copilot-swe-agent[bot]
cb0d8a38f1 Fix stack-use-after-return in client_routes delete/set functions
Change lambda captures in set_client_routes and delete_client_routes to move
route_keys/route_entries into the inner lambda instead of capturing by reference.
This prevents use-after-return when the with_retry coroutine suspends.

Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
2025-12-21 07:54:34 +00:00
copilot-swe-agent[bot]
12787302bf Initial plan 2025-12-21 07:50:24 +00:00
3 changed files with 16 additions and 33 deletions

View File

@@ -6,8 +6,6 @@ on:
jobs:
call-jira-status-in-review:
permissions:
contents: read
uses: scylladb/github-automation/.github/workflows/main_update_jira_status_to_in_review.yml@main
secrets:
caller_jira_auth: ${{ secrets.USER_AND_KEY_FOR_JIRA_AUTOMATION }}

View File

@@ -105,7 +105,7 @@ seastar::future<> service::client_routes_service::delete_client_routes_inner(con
seastar::future<> service::client_routes_service::set_client_routes(const std::vector<service::client_routes_service::client_route_entry>& route_entries) {
return container().invoke_on(0, [route_entries = std::move(route_entries)] (service::client_routes_service& cr) -> future<> {
return cr.with_retry([&] {
return cr.with_retry([&cr, route_entries = std::move(route_entries)] () mutable {
return cr.set_client_routes_inner(route_entries);
});
});
@@ -113,7 +113,7 @@ seastar::future<> service::client_routes_service::set_client_routes(const std::v
seastar::future<> service::client_routes_service::delete_client_routes(const std::vector<service::client_routes_service::client_route_key>& route_keys) {
return container().invoke_on(0, [route_keys = std::move(route_keys)] (service::client_routes_service& cr) -> future<> {
return cr.with_retry([&] {
return cr.with_retry([&cr, route_keys = std::move(route_keys)] () mutable {
return cr.delete_client_routes_inner(route_keys);
});
});

View File

@@ -198,62 +198,47 @@ SEASTAR_TEST_CASE(vector_store_client_test_dns_resolving_repeated) {
auto cfg = config();
cfg.vector_store_primary_uri.set("http://good.authority.here:6080");
auto vs = vector_store_client{cfg};
bool fail_dns_resolution = true;
auto count = 0;
auto as = abort_source_timeout();
auto address = inet_address("127.0.0.1");
configure(vs)
.with_dns_refresh_interval(milliseconds(10))
.with_wait_for_client_timeout(milliseconds(20))
.with_dns_resolver([&](auto const& host) -> future<std::optional<inet_address>> {
.with_dns_resolver([&count](auto const& host) -> future<std::optional<inet_address>> {
BOOST_CHECK_EQUAL(host, "good.authority.here");
if (fail_dns_resolution) {
count++;
if (count % 3 != 0) {
co_return std::nullopt;
}
co_return address;
co_return inet_address(format("127.0.0.{}", count));
});
vs.start_background_tasks();
// Wait for the DNS resolution to fail
BOOST_CHECK(co_await repeat_until(seconds(1), [&vs, &as]() -> future<bool> {
auto addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
co_return addrs.empty();
}));
fail_dns_resolution = false;
// Wait for the DNS resolution to succeed
BOOST_CHECK(co_await repeat_until(seconds(1), [&vs, &as]() -> future<bool> {
auto addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
co_return addrs.size() == 1;
}));
auto addrs1 = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
BOOST_REQUIRE_EQUAL(addrs1.size(), 1);
BOOST_CHECK_EQUAL(print_addr(addrs1[0]), "127.0.0.1");
BOOST_CHECK_EQUAL(count, 3);
auto addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
BOOST_REQUIRE_EQUAL(addrs.size(), 1);
BOOST_CHECK_EQUAL(print_addr(addrs[0]), "127.0.0.3");
fail_dns_resolution = true;
// Trigger DNS resolver to check for address changes
// Resolver will not re-check automatically after successful resolution
vector_store_client_tester::trigger_dns_resolver(vs);
// Wait for the DNS resolution to fail again
BOOST_CHECK(co_await repeat_until(seconds(1), [&vs, &as]() -> future<bool> {
auto addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
co_return addrs.empty();
}));
// Resolve to a different address
address = inet_address("127.0.0.2");
fail_dns_resolution = false;
// Wait for the DNS resolution to succeed
BOOST_CHECK(co_await repeat_until(seconds(1), [&vs, &as]() -> future<bool> {
auto addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
co_return addrs.size() == 1;
}));
auto addrs2 = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
BOOST_REQUIRE_EQUAL(addrs2.size(), 1);
BOOST_CHECK_EQUAL(print_addr(addrs2[0]), "127.0.0.2");
BOOST_CHECK_EQUAL(count, 6);
addrs = co_await vector_store_client_tester::resolve_hostname(vs, as.reset());
BOOST_REQUIRE_EQUAL(addrs.size(), 1);
BOOST_CHECK_EQUAL(print_addr(addrs[0]), "127.0.0.6");
co_await vs.stop();
}