#35676 util: Abort in CheckDiskSpace/FlatFileSeq::Open on rare exceptions
https://github.com/bitcoin/bitcoin/pull/35676 · · +155/-3 in 6 files, 2 commits · labels: Utils/log/libs
Goal
- Log descriptive error messages when disk space checks fail instead of crashing without diagnostics
- Ensures node operators can diagnose termination caused by disconnected or failing storage media
Catches std::error_code during std::filesystem::space and create_directories in CheckDiskSpace and FlatFileSeq::Open, writes an error message to stderr and the debug log, and calls std::abort(). Also adds a functional test simulating I/O failures across background scheduler checks and index file allocations.
Problem: Uncaught filesystem_error exceptions during disk space checks in background threads (such as scheduler checks or block filter index allocations) trigger unhandled exception termination without descriptive logging when storage media becomes unavailable.
Category: Utilities (logging, arguments, libraries) (#48 of 66)
P3 · stability
- P3 because prevents silent crashes on filesystem errors but occurs only in rare environment conditions
- Node operators gain clearer diagnostics when storage fails, but regular operation is unaffected
Addresses unhandled exceptions in shared disk space helpers during rare filesystem faults, adding explicit error diagnostic logging prior to exit. While preventing silent termination without error messages is worthwhile, the problem occurs only during abnormal environment conditions like disconnected volumes and can be deferred.
Membership: Modifies shared filesystem helper CheckDiskSpace in src/util/fs_helpers.cpp and command line test options in src/common/args.cpp.
Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0
Reviewability: Stale: Author silent 64 days
- Stale: author silent for over 60 days while fundamental approach objections remain unaddressed
The author has not commented or pushed updates since 2026-07-15, exceeding the 60-day project staleness threshold while approach objections remain unaddressed.
Author status: silent since 2026-07-15
Open concerns:
- Low-level utility helpers should not make process-wide termination policy decisions; errors or exceptions should propagate up to subsystem boundaries (josibake, optout21).
- Calling std::abort() abruptly prevents other subsystems like the wallet or chainstate from cleanly flushing to disk, which is dangerous if they reside on separate healthy storage (furszy).
- Thread boundaries should catch runtime exceptions and trigger an orderly graceful shutdown rather than aborting directly in helper functions (sedited).
Resolved concerns:
- Missing error detail: initial iteration did not log the specific OS filesystem error code before failing (raised by sedited and furszy, addressed by logging the error code before aborting).
Agreement: Disputed
- Disputed over whether low-level utility helpers should abort or propagate errors
- Unaddressed objection: low-level helpers should not make process termination decisions (josibake, optout21)
- Unaddressed objection: abrupt abort prevents clean flushes of wallet and chainstate data (furszy)
- Prefers catching exceptions at thread boundaries to trigger graceful shutdown (sedited)
Disputed: reviewers object to aborting inside low-level helpers rather than propagating errors for clean shutdown
Multiple reviewers raised architectural objections to calling std::abort() inside low-level filesystem primitives, arguing that errors should propagate to allow clean state flushes, while the author argued that rare I/O errors are comparable to OOM kills and do not justify complex error plumbing.
- josibake: 'repeating the same bad pattern of having a policy decision being made in a low level primitive... caller then decides'
- furszy: 'I don't think a low-level unclean shutdown is the right approach. An error accessing an index directory or another unrelated file shouldn't prevent the chain or wallet from attempting to flush their state to disk.'
- sedited: 'In my opinion this is most clearly solved by throwing exceptions and catching them as close to the module/thread boundaries as possible.'
- optout21: 'weak inclination towards N.A.C.K-ing the approach, on the grounds that higher-level context-aware error handling is preferable against low-level context-free abort.'
Review verdicts (DrahtBot): 0
Files
131 lines under test/bench/ci.
- test/functional/feature_io_errors.py +130/-0
- src/util/fs_helpers.cpp +11/-1
- src/flatfile.cpp +10/-1
- src/index/blockfilterindex.cpp +2/-1
- src/common/args.cpp +1/-0
- test/functional/test_runner.py +1/-0
Card
This PR modifies CheckDiskSpace and FlatFileSeq::Open to log the system error code and invoke std::abort() when filesystem operations fail unexpectedly, accompanied by a new functional test simulating volume detachments. It addresses unexpected termination via unhandled filesystem_error exceptions in background threads when a drive becomes unavailable. Multiple reviewers (josibake, furszy, sedited, optout21) dispute the design, maintaining that low-level helpers should not abort the process directly and should instead allow callers or thread boundaries to attempt clean state flushes. The PR is currently stale, with the author silent for over 60 days.