mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-06-03 13:37:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge 'utils: chunked_vector: fill ctor: make exception safe' from Benny Halevy

Browse Source 
Currently, if the fill ctor throws an exception,
the destructor won't be called, as it object is not fully constructed yet.

Call the default ctor first (which doesn't throw)
to make sure the destructor will be called on exception.

Fixes scylladb/scylladb#18635

- [x] Although the fixes is for a rare bug, it has very low risk and so it's worth backporting to all live versions

Closes scylladb/scylladb#18636

* github.com:scylladb/scylladb:
  chunked_vector_test: add more exception safety tests
  chunked_vector_test: exception_safe_class: count also moved objects
  utils: chunked_vector: fill ctor: make exception safe
This commit is contained in:
Botond Dénes
2024-05-14 13:35:02 +03:00
parent 448f651049 4bbb66f805
commit a15a9c3e8d
2 changed files with 128 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
test/boost/chunked_vector_test.cc
View File

@@ -8,6 +8,9 @@
#define BOOST_TEST_MODULE core
#include <stdexcept>
#include <fmt/format.h>
#include <boost/test/included/unit_test.hpp>
#include <deque>
#include <random>
@@ -108,12 +111,15 @@ BOOST_AUTO_TEST_CASE(test_random_walk) {
}
class exception_safety_checker {
uint64_t _live_objects = 0;
uint64_t _countdown = std::numeric_limits<uint64_t>::max();
int64_t _live_objects = 0;
int64_t _countdown = std::numeric_limits<int64_t>::max();
public:
bool ok() const {
return !_live_objects;
}
int64_t live_objects() const {
return _live_objects;
}
void set_countdown(unsigned x) {
_countdown = x;
}
@@ -123,6 +129,9 @@ public:
}
++_live_objects;
}
void add_live_object_noexcept() noexcept {
++_live_objects;
}
void del_live_object() {
--_live_objects;
}
@@ -137,7 +146,9 @@ public:
exception_safe_class(const exception_safe_class& x) : _esc(x._esc) {
_esc.add_live_object();
}
exception_safe_class(exception_safe_class&&) = default;
exception_safe_class(exception_safe_class&& x) noexcept : _esc(x._esc) {
_esc.add_live_object_noexcept();
}
~exception_safe_class() {
_esc.del_live_object();
}
@@ -290,3 +301,115 @@ BOOST_AUTO_TEST_CASE(test_push_back_using_existing_element) {
do_test_push_back_using_existing_element<chunked_vector_type>([] (chunked_vector_type& v, const push_back_item& x) { v.emplace_back(x); },
chunked_vector_type::max_chunk_capacity() + 2);
}
BOOST_AUTO_TEST_CASE(tests_insertion_exception_safety) {
constexpr size_t chunk_size = 512;
using chunked_vector = utils::chunked_vector<exception_safe_class, chunk_size>;
constexpr size_t max_chunk_capacity = chunked_vector::max_chunk_capacity();
// FIXME: convert to seastar test infstrature and use test::random
// for reproducibility
std::random_device r;
auto seed = r();
BOOST_TEST_MESSAGE(fmt::format("random-seed={}", seed));
auto rand = std::default_random_engine(seed);
auto size_dist = std::uniform_int_distribution<size_t>(1, 4 * max_chunk_capacity);
auto checker = exception_safety_checker();
auto count = size_dist(rand);
BOOST_TEST_MESSAGE(fmt::format("count={}", count));
checker.set_countdown(count - 1);
try {
chunked_vector v;
for (size_t i = 0; i < count; i++) {
v.emplace_back(checker);
}
BOOST_REQUIRE(false);
} catch (...) {
BOOST_REQUIRE_EQUAL(checker.live_objects(), 0);
}
}
BOOST_AUTO_TEST_CASE(tests_insertion_exception_safety_with_reserve) {
constexpr size_t chunk_size = 512;
using chunked_vector = utils::chunked_vector<exception_safe_class, chunk_size>;
constexpr size_t max_chunk_capacity = chunked_vector::max_chunk_capacity();
// FIXME: convert to seastar test infstrature and use test::random
// for reproducibility
std::random_device r;
auto seed = r();
BOOST_TEST_MESSAGE(fmt::format("random-seed={}", seed));
auto rand = std::default_random_engine(seed);
auto size_dist = std::uniform_int_distribution<size_t>(1, 4 * max_chunk_capacity);
auto count = size_dist(rand);
BOOST_TEST_MESSAGE(fmt::format("count={}", count));
auto checker = exception_safety_checker();
checker.set_countdown(count - 1);
try {
chunked_vector v;
auto reserve_count = size_dist(rand);
BOOST_TEST_MESSAGE(fmt::format("reserve_count={}", reserve_count));
v.reserve(reserve_count);
for (size_t i = 0; i < count; i++) {
v.emplace_back(checker);
}
BOOST_REQUIRE(false);
} catch (...) {
BOOST_REQUIRE_EQUAL(checker.live_objects(), 0);
}
}
// Reproduces https://github.com/scylladb/scylladb/issues/18635
BOOST_AUTO_TEST_CASE(tests_fill_constructor_exception_safety) {
constexpr size_t chunk_size = 512;
using chunked_vector = utils::chunked_vector<exception_safe_class, chunk_size>;
constexpr size_t max_chunk_capacity = chunked_vector::max_chunk_capacity();
// FIXME: convert to seastar test infstrature and use test::random
// for reproducibility
std::random_device r;
auto seed = r();
BOOST_TEST_MESSAGE(fmt::format("random-seed={}", seed));
auto rand = std::default_random_engine(seed);
auto size_dist = std::uniform_int_distribution<size_t>(1, 4 * max_chunk_capacity);
auto count = size_dist(rand);
BOOST_TEST_MESSAGE(fmt::format("count={}", count));
auto checker = exception_safety_checker();
auto filler = std::optional<exception_safe_class>(checker);
checker.set_countdown(count - 1);
try {
chunked_vector v(count, *filler);
BOOST_REQUIRE(false);
} catch (...) {
filler.reset();
BOOST_REQUIRE_EQUAL(checker.live_objects(), 0);
}
}
BOOST_AUTO_TEST_CASE(tests_copy_constructor_exception_safety) {
constexpr size_t chunk_size = 512;
using chunked_vector = utils::chunked_vector<exception_safe_class, chunk_size>;
constexpr size_t max_chunk_capacity = chunked_vector::max_chunk_capacity();
// FIXME: convert to seastar test infstrature and use test::random
// for reproducibility
std::random_device r;
auto seed = r();
BOOST_TEST_MESSAGE(fmt::format("random-seed={}", seed));
auto rand = std::default_random_engine(seed);
auto size_dist = std::uniform_int_distribution<size_t>(1, 4 * max_chunk_capacity);
auto count = size_dist(rand);
BOOST_TEST_MESSAGE(fmt::format("count={}", count));
auto checker = exception_safety_checker();
chunked_vector src(count, exception_safe_class(checker));
checker.set_countdown(count - 1);
try {
chunked_vector v(src);
BOOST_REQUIRE(false);
} catch (...) {
src.clear();
BOOST_REQUIRE_EQUAL(checker.live_objects(), 0);
}
}

3
utils/chunked_vector.hh
View File

@@ -351,7 +351,8 @@ chunked_vector<T, max_contiguous_allocation>::chunked_vector(Iterator begin, Ite
}
template <typename T, size_t max_contiguous_allocation>
chunked_vector<T, max_contiguous_allocation>::chunked_vector(size_t n, const T& value) {
chunked_vector<T, max_contiguous_allocation>::chunked_vector(size_t n, const T& value)
: chunked_vector() {
reserve(n);
std::fill_n(std::back_inserter(*this), n, value);
}