streams: avoid termination on buffered write failure #36049

pull l0rinc wants to merge 3 commits into bitcoin:master from l0rinc:l0rinc/streams-buffered-write-failure changing 3 files +20 −3
  1. l0rinc commented at 10:06 PM on August 20, 2026: contributor

    Problem: Block and undo writers buffer serialized data, then rely on BufferedWriter's destructor to write the remaining bytes. fwrite() can report fewer bytes written than requested after a local storage error, e.g. if the filesystem fills up while a block is being written (my 1 TB benchmarking servers have recently been filling up, so I've been seeing these failures more often). AutoFile::write_buffer() converts that result into an exception and because the destructor is implicitly non-throwing, the exception invokes std::terminate instead of reaching normal storage-error handling. Calling flush() before destruction is insufficient on its own because a failed flush leaves the same bytes pending, so the destructor retries the write while the original exception unwinds.

    I introduced this bug in #31551 when adding the BufferedWriter.

    Fix: BufferedWriter now clears the pending byte count before writing, preventing a destructor retry after a failed explicit flush. Block and undo writers flush before destruction, so write failures follow normal storage-error handling instead of terminating the process.

    <details> <summary>Manual short-write reproducer</summary>

    sed -i '' '122s/src.size()/0/' src/streams.cpp
    cmake -B build && cmake --build build -j && build/bin/bitcoind -regtest -datadir="$(mktemp -d)"; echo "exit=$?"
    

    With implicit flushing, bitcoind aborts:

    libc++abi: terminating due to uncaught exception of type std::__1::ios_base::failure: AutoFile::write_buffer: write failed: unspecified iostream_category error zsh: abort build/bin/bitcoind -regtest -datadir="$(mktemp -d)" exit=134

    With explicit flushing, it reports Failed to write genesis block and exits 1 instead of terminating the process:

    Shutdown done exit=1

    </details>

  2. test: characterize failed buffered flush
    A failed explicit `BufferedWriter` flush leaves bytes pending, so the destructor retries the write while the exception unwinds.
    
    Record both writes while allowing the retry to succeed. This establishes the behavior that must change before callers can safely flush before destruction.
    2a5d83f578
  3. streams: do not retry failed flushes
    A failed `BufferedWriter::flush()` leaves bytes pending, so the destructor retries the write while the exception unwinds.
    
    Clear the pending byte count before writing to the underlying stream. The original exception can then propagate without another write.
    cd8ea7ebdc
  4. blockstorage: handle buffered write failures
    `BufferedWriter`'s destructor is implicitly non-throwing, so a failure while it writes pending block or undo bytes invokes `std::terminate` before normal exception handling can report it.
    
    Flush block and undo writers before destruction. Write failures can then follow the existing error paths and produce a controlled storage failure.
    d5f424103b
  5. DrahtBot commented at 10:06 PM on August 20, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36049.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  6. andrewtoth commented at 10:21 PM on August 20, 2026: contributor

    the exception invokes std::terminate instead of reaching normal storage-error handling.

    Can you expand on why this is a problem? If there is a storage error, don't we have to crash anyways?

  7. l0rinc commented at 10:35 PM on August 20, 2026: contributor

    If there is a storage error, don't we have to crash anyways?

    Yes, the node still has to stop, but std::terminate bypasses the existing error handling.

    Can you expand on why this is a problem?

    Handling the exception lets the node report the storage error and run its normal shutdown cleanup.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-21 04:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me