Skip to content
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.

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

create_scheduling_group / scheduling_group_key_create not exception safe when SG key data constructor throws #2222

Open
piodul opened this issue May 6, 2024 · 0 comments
Assignees

Comments

@piodul
Copy link
Contributor

piodul commented May 6, 2024

There are two cases where data for a scheduling group key can be constructed:

  • When the SG key already exists and a new SG is created (create_scheduling_group)
  • When a new SG key is being created (scheduling_group_key_create)

In both cases, the SG key data constructor can fail. This is an example test for the second case:

SEASTAR_THREAD_TEST_CASE(sg_key_constructor_exception_when_creating_new_key) {
    struct thrower {
        thrower() {
            throw std::runtime_error("oopsie daisy");
        }
        ~thrower() {
            // Shouldn't get here because the constructor shouldn't succeed
            BOOST_ASSERT(false);
        }
    };
    scheduling_group_key_config thrower_conf = make_scheduling_group_key_config<thrower>();
    BOOST_REQUIRE_THROW(scheduling_group_key_create(thrower_conf).get(), std::runtime_error);
}

Even though the constructor of thrower always throws, its destructor is called anyway:

scheduling_group_test: /home/piodul/code/seastar/tests/unit/scheduling_group_test.cc:392: sg_key_constructor_exception_when_creating_new_key::do_run_test_case()::thrower::~thrower(): Assertion `false' failed.
scheduling_group_test: /home/piodul/code/seastar/tests/unit/scheduling_group_test.cc:392: sg_key_constructor_exception_when_creating_new_key::do_run_test_case()::thrower::~thrower(): Assertion `false' failed.
scheduling_group_test: /home/piodul/code/seastar/tests/unit/scheduling_group_test.cc:392: sg_key_constructor_exception_when_creating_new_key::do_run_test_case()::thrower::~thrower(): Assertion `false' failed.
Aborted (core dumped)

It looks like both functions are not exception safe and cause issues later if the SG key data constructor fails with an exception. I believe that both operations should be able to roll back in case of an exception.

@piodul piodul self-assigned this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant