Skip to content

Commit

Permalink
testing: prevent test scheduling after reactor exit
Browse files Browse the repository at this point in the history
Previously, when handling "--help" (introduced in b8a13be), we returned
status code 0 but continued scheduling test tasks to the Seastar reactor.
This caused test applications to hang since the tasks expected the exchanger
'e' to be available after reactor exit.

Fix this by using the "_done" flag to track reactor state:
- Set "_done" when Seastar application exits
- Skip task scheduling and exchanger wait if "_done" is set
- Change test_runner::start_thread() to return bool indicating successful
  engine startup instead of exit code (exit code now stored in _exit_code
  member)

This fixes the regression where "--help" would cause test applications to
hang indefinitely.

Fixes #2635
Signed-off-by: Kefu Chai <[email protected]>

Closes #2644
  • Loading branch information
tchaikov authored and xemul committed Feb 19, 2025
1 parent 87c221c commit 9ca4fee
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion include/seastar/testing/test_runner.hh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private:
start_thread_args(int ac_, char** av_) noexcept : ac(ac_), av(av_) {}
};
std::unique_ptr<start_thread_args> _st_args;
int start_thread(int ac, char** av);
bool start_thread(int ac, char** av);
public:
// Returns whether initialization was successful.
// Will return as soon as the seastar::app was started.
Expand Down
14 changes: 7 additions & 7 deletions src/testing/test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ test_runner::start(int ac, char** av) {
return true;
}

int test_runner::start_thread(int ac, char** av) {
auto init_outcome = std::make_shared<exchanger<int>>();
bool test_runner::start_thread(int ac, char** av) {
auto init_outcome = std::make_shared<exchanger<bool>>();

namespace bpo = boost::program_options;
_thread = std::make_unique<posix_thread>([this, ac, av, init_outcome]() mutable {
Expand All @@ -75,7 +75,7 @@ int test_runner::start_thread(int ac, char** av) {
// We guarantee that only one thread is running.
// We only read this after that one thread is joined, so this is safe.
_exit_code = app.run(ac, av, [this, &app, init_outcome = init_outcome.get()] {
init_outcome->give(0);
init_outcome->give(true);
auto init = [&app] {
auto conf_seed = app.configuration()["random-seed"];
auto seed = conf_seed.empty() ? std::random_device()(): conf_seed.as<unsigned>();
Expand Down Expand Up @@ -108,7 +108,7 @@ int test_runner::start_thread(int ac, char** av) {
return 0;
});
});
init_outcome->give(_exit_code);
init_outcome->give(false);
});

return init_outcome->take();
Expand All @@ -119,14 +119,14 @@ test_runner::run_sync(std::function<future<>()> task) {
if (_st_args) {
start_thread_args sa = *_st_args;
_st_args.reset();
if (int start_status = start_thread(sa.ac, sa.av); start_status != 0) {
if (!start_thread(sa.ac, sa.av)) {
// something bad happened when starting the reactor or app, and
// the _thread has exited before taking any task. but we need to
// move on. let's report this bad news with exit code
_exit_code = start_status;
_done = true;
}
}
if (_exit_code != 0) {
if (_done) {
// we failed to start the worker reactor, so we cannot send the task to
// it.
return;
Expand Down

0 comments on commit 9ca4fee

Please sign in to comment.