Skip to content

Fix undetected buffered output-write failures - #716

Open
thierrygosselin wants to merge 2 commits into
OpenGene:masterfrom
thierrygosselin:fix-buffered-output-errors
Open

thierrygosselin wants to merge 2 commits into
OpenGene:masterfrom
thierrygosselin:fix-buffered-output-errors

Conversation

@thierrygosselin

Copy link
Copy Markdown

Summary

This PR makes the buffered Writer fail explicitly when output cannot be written completely.

Previously, writeInternal() accepted any positive fwrite() result as success, callers could ignore a failed write, and final stream errors were unchecked. This could leave truncated output while fastp returned exit status 0.

Changes

  • Check complete writes and stream errors.
  • Fail explicitly on compression-buffer allocation or compression failure.
  • Check fclose() errors.
  • Check final flushing of stdout without closing it.
  • Include the output destination in error messages.

The separate multithreaded gzip pwrite implementation is unchanged.

Regression tests

Added scripts/test_buffered_output_errors.py, covering plain FASTQ, single-thread gzip, stdout, and split output.

The tests use synthetic reads and a process-specific file-size limit to simulate write failures without filling a disk. Reports are directed to /dev/null to isolate FASTQ output failures.

Run with:

python3 scripts/test_buffered_output_errors.py ./fastp

Tested locally on macOS ARM64:

  • Unmodified Conda fastp 1.3.6: all four normal-output cases pass; all four write-error cases incorrectly exit successfully.
  • Locally compiled patched source: all eight cases pass.
  • Successful outputs preserve the synthetic reads exactly.

Linux execution and isolated allocation/compression-failure injection have not been tested.

Check complete writes and stream errors in the buffered Writer.

Report compression and allocation failures explicitly, check fclose errors,
and check final flushing of stdout without closing it.

This prevents incomplete FASTQ output from being reported as a successful
run when an output-write failure occurs.
Test plain FASTQ, single-thread gzip, stdout, and split output.

Use a process-specific file-size limit to simulate write failures without
filling the disk. Require explicit failure diagnostics and a nonzero exit
status. Verify that successful runs preserve the synthetic reads exactly.

All eight cases pass with the writer fix. The unmodified Conda 1.3.6 build
fails the four write-error cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant