Skip to content

test: remove flaky timing dependence from the test suite #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sgerbino merged 2 commits into from
Sep 8, 2026
Merged

test: remove flaky timing dependence from the test suite #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86a0f46
test: remove flaky timing dependence from the test suite
sgerbino Sep 8, 2026
de60d31
fix(resolver): post the IOCP completion before balancing work
sgerbino Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions include/boost/corosio/native/detail/iocp/win_resolver_service.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,11 @@ resolve_op::completion(DWORD dwError, DWORD /*bytes*/, OVERLAPPED* ov)
{
auto* op = static_cast<resolve_op*>(ov);
op->dwError = dwError;
op->impl->svc_.work_finished();
op->impl->svc_.post(op);
// Post before work_finished, or the count can hit zero and run() frees
// svc_ before the post; cache svc_ since posting may free the impl.
auto& svc = op->impl->svc_;
svc.post(op);
svc.work_finished();
}

inline resolve_op::resolve_op() noexcept : overlapped_op(&do_complete) {}
Expand Down Expand Up @@ -402,11 +405,8 @@ win_resolver::resolve(
if (result != WSA_IO_PENDING)
{
// Completed synchronously - callback won't be invoked
svc_.work_finished();

if (result == 0)
{
// Completed synchronously
op.dwError = 0;
}
else
Expand All @@ -415,6 +415,7 @@ win_resolver::resolve(
}

svc_.post(&op);
svc_.work_finished();
}
// completion is always posted to scheduler queue, never inline.
return std::noop_coroutine();
Expand Down Expand Up @@ -523,13 +524,16 @@ win_resolver::do_reverse_resolve_work(pool_work_item* w) noexcept
}
}

self->svc_.work_finished();

// Hand the keepalive to the op: the completion waits in the
// scheduler's queue, and the implementation embedding it must
// outlive that wait. Nothing may touch *self after the post.
self->reverse_op_.impl_ptr = std::move(pw->ref_);
self->svc_.post(&self->reverse_op_);

// Post before work_finished (see resolve_op::completion); cache svc_
// since posting may free *self.
auto& svc = self->svc_;
svc.post(&self->reverse_op_);
svc.work_finished();
}

// win_resolver_service
Expand Down
32 changes: 9 additions & 23 deletions test/unit/io_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,16 +622,12 @@ struct io_context_test
auto ex = ioc.get_executor();
int counter = 0;

// run_for with no work - returns immediately and stops context
auto start = std::chrono::steady_clock::now();
// run_for with no work returns without blocking and stops the
// context; a wall-clock bound here only flakes under load.
std::size_t n = ioc.run_for(std::chrono::milliseconds(20));
auto elapsed = std::chrono::steady_clock::now() - start;

BOOST_TEST(n == 0);
BOOST_TEST(ioc.stopped());
auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed)
.count();
BOOST_TEST(ms < 15); // Should return immediately when no work

// Must restart before next use
ioc.restart();
Expand All @@ -651,17 +647,11 @@ struct io_context_test
// Simulate persistent outstanding work (like a listening acceptor)
ex.on_work_started();

auto start = std::chrono::steady_clock::now();
// Outstanding work keeps the context alive; run_for must return
// when its timeout elapses rather than block forever. A genuine
// hang surfaces as a harness timeout, not a wall-clock assert.
std::size_t n = ioc.run_for(std::chrono::milliseconds(200));
auto elapsed = std::chrono::steady_clock::now() - start;

auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed)
.count();

// Must return after ~200ms, not block forever
BOOST_TEST(n == 0);
BOOST_TEST(ms >= 150);
BOOST_TEST(ms < 1000);

ex.on_work_finished();
}
Expand All @@ -673,16 +663,12 @@ struct io_context_test

ex.on_work_started();

auto start = std::chrono::steady_clock::now();
// Outstanding work keeps the context alive; run_one_for must
// return when its timeout elapses rather than block forever. A
// genuine hang surfaces as a harness timeout, not a wall-clock
// assert.
std::size_t n = ioc.run_one_for(std::chrono::milliseconds(200));
auto elapsed = std::chrono::steady_clock::now() - start;

auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed)
.count();

BOOST_TEST(n == 0);
BOOST_TEST(ms >= 150);
BOOST_TEST(ms < 1000);

ex.on_work_finished();
}
Expand Down
11 changes: 4 additions & 7 deletions test/unit/native/native_io_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,12 @@ struct native_io_context_test

ex.on_work_started();

auto start = std::chrono::steady_clock::now();
// Outstanding work keeps the scheduler alive; run_for must
// return when its timeout elapses rather than block forever. A
// genuine hang surfaces as a harness timeout, not a wall-clock
// assert.
std::size_t n = ctx.run_for(std::chrono::milliseconds(50));
auto elapsed = std::chrono::steady_clock::now() - start;

BOOST_TEST(n == 0u);
auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed)
.count();
BOOST_TEST(ms >= 30);
BOOST_TEST(ms < 1000);

ex.on_work_finished();
}
Expand Down
86 changes: 19 additions & 67 deletions test/unit/tcp_acceptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1374,32 +1374,21 @@ struct tcp_acceptor_test
ioc.restart();

std::error_code wait_ec;
bool wait_done = false;
bool watchdog_fired = false;
bool wait_done = false;

// A reactor that misses pre-existing readiness parks the wait
// forever, surfacing as a harness timeout; the run only returns
// here once the wait completes on its own.
auto waiter = [&]() -> capy::task<> {
auto [wec] = co_await acc.wait(wait_type::read);
wait_ec = wec;
wait_done = true;
};
// Watchdog: a reactor that misses pre-existing readiness
// parks forever; retract the wait so the miss is reported
// instead of hanging the suite.
auto watchdog = [&]() -> capy::task<> {
std::ignore = co_await corosio::delay(std::chrono::milliseconds(250));
if (!wait_done)
{
watchdog_fired = true;
acc.cancel();
}
};
capy::run_async(ex)(waiter());
capy::run_async(ex)(watchdog());
ioc.run();
ioc.restart();

BOOST_TEST(wait_done);
BOOST_TEST(!watchdog_fired);
BOOST_TEST(!wait_ec);

// The signalled connection is genuinely acceptable.
Expand Down Expand Up @@ -1441,29 +1430,21 @@ struct tcp_acceptor_test
ioc.restart();

std::error_code wait_ec;
bool wait_done = false;
bool watchdog_fired = false;
bool wait_done = false;

// A registration that misses the already-queued backlog parks
// the wait forever, surfacing as a harness timeout; the run
// only returns here once the wait completes on its own.
auto waiter = [&]() -> capy::task<> {
auto [wec] = co_await acc.wait(wait_type::read);
wait_ec = wec;
wait_done = true;
};
auto watchdog = [&]() -> capy::task<> {
std::ignore = co_await corosio::delay(std::chrono::milliseconds(250));
if (!wait_done)
{
watchdog_fired = true;
acc.cancel();
}
};
capy::run_async(ex)(waiter());
capy::run_async(ex)(watchdog());
ioc.run();
ioc.restart();

BOOST_TEST(wait_done);
BOOST_TEST(!watchdog_fired);
BOOST_TEST(!wait_ec);

bool accepted = false;
Expand Down Expand Up @@ -1496,30 +1477,20 @@ struct tcp_acceptor_test
BOOST_TEST(!ec);

std::error_code wait_ec;
bool wait_done = false;
bool watchdog_fired = false;
bool wait_done = false;

// The wait must fail synchronously with operation_not_supported;
// a backend that instead parks the meaningless wait parks it
// forever, surfacing as a harness timeout.
auto waiter = [&]() -> capy::task<> {
auto [wec] = co_await acc.wait(wait_type::write);
wait_ec = wec;
wait_done = true;
};
// Watchdog: a backend that parks the meaningless wait would
// hang the suite; retract it so the miss is reported.
auto watchdog = [&]() -> capy::task<> {
std::ignore = co_await corosio::delay(std::chrono::milliseconds(250));
if (!wait_done)
{
watchdog_fired = true;
acc.cancel();
}
};
capy::run_async(ex)(waiter());
capy::run_async(ex)(watchdog());
ioc.run();

BOOST_TEST(wait_done);
BOOST_TEST(!watchdog_fired);
BOOST_TEST(wait_ec == std::errc::operation_not_supported);
}

Expand Down Expand Up @@ -1657,31 +1628,20 @@ struct tcp_acceptor_test
BOOST_TEST(native_connect_loopback(client, port, false));

std::error_code accept_ec;
bool accept_done = false;
bool watchdog_fired = false;
bool accept_done = false;

// A retired arming stealing the connection parks the accept
// forever, surfacing as a harness timeout; the run only returns
// here once the accept completes on its own.
auto server = [&]() -> capy::task<> {
auto [aec, peer] = co_await acc.accept();
accept_ec = aec;
accept_done = true;
};
// Watchdog: a retired arming stealing the connection parks the
// accept forever; retract it so the theft is reported instead
// of hanging the suite.
auto watchdog = [&]() -> capy::task<> {
std::ignore = co_await corosio::delay(std::chrono::milliseconds(250));
if (!accept_done)
{
watchdog_fired = true;
acc.cancel();
}
};
capy::run_async(ex)(server());
capy::run_async(ex)(watchdog());
ioc.run();

BOOST_TEST(accept_done);
BOOST_TEST(!watchdog_fired);
BOOST_TEST(!accept_ec);

close_native_socket(client);
Expand Down Expand Up @@ -1741,29 +1701,21 @@ struct tcp_acceptor_test
std::error_code accept_ec;
std::uint16_t accepted_port = 0;
bool accept_done = false;
bool watchdog_fired = false;

// A stale pre-accepted connection surfacing here would satisfy
// the accept with the wrong peer; a new listener that never
// delivers parks it forever, surfacing as a harness timeout.
auto server = [&]() -> capy::task<> {
auto [aec, peer] = co_await acc.accept();
accept_ec = aec;
accept_done = true;
if (!aec)
accepted_port = peer.local_endpoint().port();
};
auto watchdog = [&]() -> capy::task<> {
std::ignore = co_await corosio::delay(std::chrono::milliseconds(250));
if (!accept_done)
{
watchdog_fired = true;
acc.cancel();
}
};
capy::run_async(ex)(server());
capy::run_async(ex)(watchdog());
ioc.run();

BOOST_TEST(accept_done);
BOOST_TEST(!watchdog_fired);
BOOST_TEST(!accept_ec);
// The accepted connection belongs to the new listener.
BOOST_TEST_EQ(accepted_port, port_b);
Expand Down
Loading