Skip to content

fix(bcf): interpretation of indexed reader initialisation return code - #507

Open
jeeeesper wants to merge 2 commits into
rust-bio:masterfrom
jeeeesper:segfault-indexed-bcf-reader
Open

jeeeesper wants to merge 2 commits into
rust-bio:masterfrom
jeeeesper:segfault-indexed-bcf-reader

Conversation

@jeeeesper

@jeeeesper jeeeesper commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

There is a small error in the initialisation code of the bcf IndexedReader: The return status of the underlying htslib method bcf_sr_add_reader is misinterpreted. For example, if the index file is missing, the underlying method fails, but the method here still returns success. This, in turn, caused (in my case) a segmentation fault downstream when trying to access fields or read from the reader.

See here the return value, clearly != 0 is more appropriate check than >= 0.
https://github.com/samtools/htslib/blob/d94071f3dd9613ab66f177e1237660855a56f0ed/htslib/synced_bcf_reader.h#L260 (edit: update link because upstream changes)

@jeeeesper
jeeeesper force-pushed the segfault-indexed-bcf-reader branch from 33d3752 to b4e08b1 Compare February 27, 2026 15:14
@coveralls

coveralls commented Mar 2, 2026

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 22566383152

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 81.77%

Totals Coverage Status
Change from base Build 21474746563: -0.006%
Covered Lines: 2790
Relevant Lines: 3412

💛 - Coveralls

@kvg

kvg commented Sep 10, 2026

Copy link
Copy Markdown

I ran into the same bug independently (failed IndexedReader::from_url on a gs:// VCF in some code of mine running in a JupyterLab environment, resulting in a segfault that kills the notebook kernel, forcing an analysis to start over from scratch).

from_path of a missing file doesn't trigger it (existence is checked in Rust first), but from_url does. A missing file:// URL is enough to reproduce without GCS.

tbx::Reader::new has a related crash: hts_open can return NULL and the next line is hts_get_format on that pointer.

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.

3 participants