refactor(cached_block): safeguard against moving block reallocation

This commit is contained in:
Marcus Holland-Moritz 2025-03-30 14:14:07 +02:00
parent 659fba3ed3
commit 1aef650477
3 changed files with 60 additions and 7 deletions

View File

@ -61,6 +61,10 @@ class mutable_byte_buffer_interface : public byte_buffer_interface {
virtual void resize(size_t size) = 0;
virtual void shrink_to_fit() = 0;
// Freezes the location of the buffer in memory, i.e. all further calls
// that would reallocate the buffer will throw.
virtual void freeze_location() = 0;
// TODO: See if we can do without this. This will *only* be implemented
// in the vector_byte_buffer, other implementations will throw.
virtual std::vector<uint8_t>& raw_vector() = 0;
@ -147,6 +151,8 @@ class mutable_byte_buffer {
void shrink_to_fit() { bb_->shrink_to_fit(); }
void freeze_location() { bb_->freeze_location(); }
std::vector<uint8_t>& raw_vector() { return bb_->raw_vector(); }
void swap(mutable_byte_buffer& other) noexcept { std::swap(bb_, other.bb_); }

View File

@ -76,7 +76,9 @@ class cached_block_ final : public cached_block {
// once the block is fully decompressed, we can reset the decompressor_
// This can be called from any thread
size_t range_end() const override { return range_end_.load(); }
size_t range_end() const override {
return range_end_.load(std::memory_order_acquire);
}
// TODO: The code relies on the fact that the data_ buffer is never
// reallocated once block decompression has started. I would like to
@ -84,7 +86,9 @@ class cached_block_ final : public cached_block {
uint8_t const* data() const override { return data_.data(); }
void decompress_until(size_t end) override {
while (data_.size() < end) {
auto pos = data_.size();
while (pos < end) {
if (!decompressor_) {
DWARFS_THROW(runtime_error, "no decompressor for block");
}
@ -97,7 +101,13 @@ class cached_block_ final : public cached_block {
try_release();
}
range_end_ = data_.size();
if (pos == 0) {
// Freeze the location of the data buffer
data_.freeze_location();
}
pos = data_.size();
range_end_.store(pos, std::memory_order_release);
}
}

View File

@ -19,6 +19,10 @@
* along with dwarfs. If not, see <https://www.gnu.org/licenses/>.
*/
#include <atomic>
#include <stdexcept>
#include <string>
#include <string_view>
#include <vector>
#include <dwarfs/vector_byte_buffer.h>
@ -55,18 +59,51 @@ class vector_byte_buffer_impl : public mutable_byte_buffer_interface {
return {data_.data(), data_.size()};
}
void clear() override { data_.clear(); }
void clear() override {
assert_not_frozen("clear");
data_.clear();
}
void reserve(size_t size) override { data_.reserve(size); }
void reserve(size_t size) override {
assert_not_frozen("reserve");
data_.reserve(size);
}
void resize(size_t size) override { data_.resize(size); }
void resize(size_t size) override {
if (frozen() && size > data_.capacity()) {
frozen_error("resize beyond capacity");
}
data_.resize(size);
}
void shrink_to_fit() override { data_.shrink_to_fit(); }
void shrink_to_fit() override {
assert_not_frozen("shrink_to_fit");
data_.shrink_to_fit();
}
void freeze_location() override {
frozen_.store(true, std::memory_order_release);
}
std::vector<uint8_t>& raw_vector() override { return data_; }
private:
void assert_not_frozen(std::string_view what) const {
if (frozen()) {
frozen_error(what);
}
}
void frozen_error(std::string_view what) const {
throw std::runtime_error("operation not allowed on frozen buffer: " +
std::string{what});
}
bool frozen() const { return frozen_.load(std::memory_order_acquire); }
std::vector<uint8_t> data_;
std::atomic<bool> frozen_{false};
static_assert(std::atomic<bool>::is_always_lock_free);
};
} // namespace