sstable: file_writer: auto-close in destructor

Otherwise we may trip the following
    assert(_closing_state == state::closed);
in ~append_challenged_posix_file_impl
when the output_stream is destructed.

Example stack trace:
    non-virtual thunk to seastar::append_challenged_posix_file_impl::~append_challenged_posix_file_impl() at /jenkins/slave/workspace/scylla-3.2/build/scylla/seastar/include/seastar/core/future.hh:944
    seastar::shared_ptr_count_for<checked_file_impl>::~shared_ptr_count_for() at crtstuff.c:?
    seastar::shared_ptr<seastar::file_impl>::~shared_ptr() at /jenkins/slave/workspace/scylla-3.2/build/scylla/seastar/include/seastar/core/future.hh:944
     (inlined by) seastar::file::~file() at /jenkins/slave/workspace/scylla-3.2/build/scylla/seastar/include/seastar/core/file.hh:155
     (inlined by) seastar::file_data_sink_impl::~file_data_sink_impl() at /jenkins/slave/workspace/scylla-3.2/build/scylla/seastar/src/core/fstream.cc:312
     (inlined by) seastar::file_data_sink_impl::~file_data_sink_impl() at /jenkins/slave/workspace/scylla-3.2/build/scylla/seastar/src/core/fstream.cc:312
    seastar::output_stream<char>::~output_stream() at crtstuff.c:?
    sstables::sstable::write_crc(sstables::checksum const&) [clone .cold] at sstables.cc:?
    sstables::mc::writer::close_data_writer() at crtstuff.c:?
    sstables::mc::writer::consume_end_of_stream() at crtstuff.c:?
    sstables::sstable::write_components(flat_mutation_reader, unsigned long, seastar::lw_shared_ptr<schema const>, sstables::sstable_writer_config const&, encoding_stats, seastar::io_priority_class const&)::{lambda()#1}::operator()() at sstables.cc:?

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
This commit is contained in:
Benny Halevy
2019-12-22 10:09:03 +02:00
parent d277ec2ab9
commit 60873d2360
2 changed files with 28 additions and 0 deletions

View File

@@ -939,9 +939,25 @@ void sstable::generate_toc(compressor_ptr c, double filter_fp_chance) {
_recognized_components.insert(component_type::Scylla);
}
file_writer::~file_writer() {
if (_closed) {
return;
}
try {
// close() should be called by the owner of the file_writer.
// However it may not be called on exception handling paths
// so auto-close the output_stream so it won't be destructed while open.
_out.close().get();
} catch (...) {
sstlog.warn("Error while auto-closing {}: {}. Ignored.", get_filename(), std::current_exception());
}
}
void file_writer::close() {
assert(!_closed && "file_writer already closed");
try {
_out.close().get();
_closed = true;
} catch (...) {
auto e = std::current_exception();
sstlog.error("Error while closing {}: {}", get_filename(), e);

View File

@@ -43,6 +43,7 @@ class file_writer {
output_stream<char> _out;
writer_offset_tracker _offset;
std::optional<sstring> _filename;
bool _closed = false;
public:
// Closes the file if file_writer creation fails
static future<file_writer> make(file f, file_output_stream_options options, sstring filename) noexcept;
@@ -56,6 +57,17 @@ public:
: _out(std::move(out))
{}
// Must be called in a seastar thread.
virtual ~file_writer();
file_writer(const file_writer&) = delete;
file_writer(file_writer&& x) noexcept
: _out(std::move(x._out))
, _offset(std::move(x._offset))
, _filename(std::move(x._filename))
, _closed(x._closed)
{
x._closed = true; // don't auto-close in destructor
}
// Must be called in a seastar thread.
void write(const char* buf, size_t n) {
_offset.offset += n;