diff --git a/src/streams.cpp b/src/streams.cpp index 19c2b474452..2c873e50d3b 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -85,6 +85,7 @@ void AutoFile::write(std::span src) if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { throw std::ios_base::failure("AutoFile::write: write failed"); } + m_was_written = true; if (m_position.has_value()) *m_position += src.size(); } else { std::array buf; @@ -107,6 +108,7 @@ void AutoFile::write_buffer(std::span src) if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { throw std::ios_base::failure("AutoFile::write_buffer: write failed"); } + m_was_written = true; if (m_position) *m_position += src.size(); } @@ -117,6 +119,7 @@ bool AutoFile::Commit() bool AutoFile::Truncate(unsigned size) { + m_was_written = true; return ::TruncateFile(m_file, size); } diff --git a/src/streams.h b/src/streams.h index 5a596047121..27d25220fbe 100644 --- a/src/streams.h +++ b/src/streams.h @@ -6,10 +6,13 @@ #ifndef BITCOIN_STREAMS_H #define BITCOIN_STREAMS_H +#include #include #include #include +#include #include +#include #include #include @@ -386,7 +389,13 @@ public: * * Will automatically close the file when it goes out of scope if not null. * If you're returning the file pointer, return file.release(). - * If you need to close the file early, use file.fclose() instead of fclose(file). + * If you need to close the file early, use autofile.fclose() instead of fclose(underlying_FILE). + * + * @note If the file has been written to, then the caller must close it + * explicitly with the `fclose()` method, check if it returns an error and treat + * such an error as if the `write()` method failed. The OS's `fclose(3)` may + * fail to flush to disk data that has been previously written, rendering the + * file corrupt. */ class AutoFile { @@ -394,11 +403,28 @@ protected: std::FILE* m_file; std::vector m_xor; std::optional m_position; + bool m_was_written{false}; public: explicit AutoFile(std::FILE* file, std::vector data_xor={}); - ~AutoFile() { fclose(); } + ~AutoFile() + { + if (m_was_written) { + // Callers that wrote to the file must have closed it explicitly + // with the fclose() method and checked that the close succeeded. + // This is because here in the destructor we have no way to signal + // errors from fclose() which, after write, could mean the file is + // corrupted and must be handled properly at the call site. + // Destructors in C++ cannot signal an error to the callers because + // they do not return a value and are not allowed to throw exceptions. + Assume(IsNull()); + } + + if (fclose() != 0) { + LogError("Failed to close file: %s", SysErrorString(errno)); + } + } // Disallow copies AutoFile(const AutoFile&) = delete; @@ -406,7 +432,7 @@ public: bool feof() const { return std::feof(m_file); } - int fclose() + [[nodiscard]] int fclose() { if (auto rel{release()}) return std::fclose(rel); return 0;