Zero the external array buffer before it is written to the file - #335
Open
glennhickey wants to merge 1 commit into
Open
Zero the external array buffer before it is written to the file#335glennhickey wants to merge 1 commit into
glennhickey wants to merge 1 commit into
Conversation
Not every byte of an Hdf5ExternalArray buffer gets set before the buffer is flushed. The bottom segment "length" field never is -- lengths are derived from the next segment's start position, so nothing writes it and nothing reads it -- and the buffer comes from new char[], so whatever the allocator last had there is serialised into the file. The result is that two runs producing the same alignment write different files. It showed up as heap garbage in element 0 of a genome's BOTTOM_ARRAY: one run had 0, another had 7378429340001763376, whose bytes spell out a fragment of a sequence name left over in freed memory. Only the first buffer is affected in a fresh file, since page() reads every later window back and hdf5 returns its fill value for regions never written. Zero it in initBuf() so it covers load() as well as create(). A file opened for modification reaches its arrays through Hdf5Genome::read(), which loads them, and segments are then written through those buffers -- zeroing only on the create() path leaves uninitialised bytes in the merged output of halAppendSubtree. Beyond reproducibility this stops process memory leaking into a shared file, and lets h5diff and checksums be used to compare two hal files meaningfully. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017E9w7KrbFEfqNRtkam4u1F
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
see ComparativeGenomicsToolkit/cactus#1985
Not every byte of an Hdf5ExternalArray buffer gets set before the buffer is flushed. The bottom segment "length" field never is -- lengths are derived from the next segment's start position, so nothing writes it and nothing reads it -- and the buffer comes from new char[], so whatever the allocator last had there is serialised into the file.