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

guard against throwing in random_access_file's write_datastream dtor #439

Merged
merged 1 commit into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions libraries/libfc/include/fc/io/random_access_file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ read_ds() and write_ds() may be called from multiple threads simultaneously. The
fc's pack/unpack. Multiple read_datastream and write_datastream created from the same random_access_file can be used simultaneously from different
threads, but an individual read/write_datastream must only be used by a single thread at a time. unpack_from() and pack_to() are just
simple helper functions based on these read/write_datastreams. Be aware that read/write_datastreams are buffered. Write buffers are flushed
as needed, both upon destruction of the datastream and also when an internal buffer becomes full.
on an internal threshold, upon a call to flush(), and upon destruction of the datastream. If a buffered write fails during destruction an
exception is NOT thrown. So if acting upon write failure is important call flush() prior to destruction; flush() does throw on failure.

seekable_device() may be called from multiple threads simultaneously. This returns a Boost Iostreams SeekableDevice. Similar to datastreams,
multiple devices created from the same random_access_file can be used simultaneously from different threads, but an individual
Expand Down Expand Up @@ -320,7 +321,13 @@ class random_access_file {
}

void do_write() {
ctx->write_to(buffer.cdata(), next_pos);
try {
ctx->write_to(buffer.cdata(), next_pos);
} catch(...) {
//prevent another write attempt during dtor
buffer.clear();
throw;
}
if(next_pos != impl::append_t)
next_pos += buffer.size();
buffer.clear();
Expand All @@ -342,12 +349,19 @@ class random_access_file {
return true;
}

void flush() {
do_write();
}

inline bool put(char c) {
return write(&c, sizeof(char));
}

~write_datastream() {
if(ctx)
do_write();
try {
if(ctx)
do_write();
} FC_LOG_AND_DROP(("write failure ignored"));
}

private:
Expand Down Expand Up @@ -419,12 +433,14 @@ class random_access_file {
void pack_to(const T& v, const ssize_t offset) {
write_datastream ds(ctx, offset);
fc::raw::pack(ds, v);
ds.flush();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the ctx->write_to() on each pack_to() call hurt performance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this case: the buffer is/was flushed upon destruction of write_datastream so the buffer was already being flushed at the end of pack_to().

}

template<typename T>
void pack_to_end(const T& v) {
write_datastream ds(ctx, impl::append_t);
fc::raw::pack(ds, v);
ds.flush();
}

read_datastream read_ds(const ssize_t offset) {
Expand Down
2 changes: 2 additions & 0 deletions libraries/state_history/include/eosio/state_history/log.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ class state_history_log {
const uint32_t num_blocks_in_log = _end_block - _begin_block;
fc::raw::pack(appender, num_blocks_in_log);
}

appender.flush();
}

std::optional<chain::block_id_type> get_block_id(uint32_t block_num) {
Expand Down