Skip to content

memory correctness #374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 29, 2025
Merged

memory correctness #374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bddcf89
Fix uninitialized boolean variables in Parameters struct
ekg Jul 29, 2025
2e7e3ec
Fix memory leaks in thread-local faidx readers
ekg Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions src/map/include/computeMap.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,15 +503,25 @@ namespace skch
exit(1);
}

// Thread-local reader wrapper that ensures cleanup
struct ReaderWrapper {
faidx_reader_t* reader = nullptr;
~ReaderWrapper() {
if (reader) {
faidx_reader_destroy(reader);
}
}
};

auto getThreadLocalReader = [](faidx_meta_t* meta) -> faidx_reader_t* {
thread_local faidx_reader_t* reader = nullptr;
if (reader == nullptr) {
reader = faidx_reader_create(meta);
if (!reader) {
thread_local ReaderWrapper wrapper;
if (wrapper.reader == nullptr) {
wrapper.reader = faidx_reader_create(meta);
if (!wrapper.reader) {
throw std::runtime_error("Failed to create thread-local reader");
}
}
return reader;
return wrapper.reader;
};

for (const auto& queryName : querySequenceNames) {
Expand Down
32 changes: 16 additions & 16 deletions src/map/include/map_parameters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,32 +39,32 @@ struct Parameters
int alphabetSize; //alphabet size
offset_t referenceSize; //Approximate reference size
float percentageIdentity; //user defined threshold for good similarity
bool stage2_full_scan; //Instead of using the best intersection for a given candidate region, compute the minhash for every position in the window
bool stage1_topANI_filter; //Use the ANI filter in stage 1
bool stage2_full_scan = false; //Instead of using the best intersection for a given candidate region, compute the minhash for every position in the window
bool stage1_topANI_filter = false; //Use the ANI filter in stage 1
float ANIDiff; //ANI distance threshold below best mapping to retain in stage 1 filtering
float ANIDiffConf; //Confidence of stage 1 ANI filtering threshold
int filterMode; //filtering mode in mashmap
uint32_t numMappingsForSegment; //how many mappings to retain for each segment
uint32_t numMappingsForShortSequence; //how many secondary alignments we keep for reads < segLength
bool dropRand; //drop mappings w/ same score until only numMappingsForSegment remain
bool dropRand = false; //drop mappings w/ same score until only numMappingsForSegment remain
int threads; //execution thread count
std::vector<std::string> refSequences; //reference sequence(s)
std::vector<std::string> querySequences; //query sequence(s)
std::string outFileName; //output file name
stdfs::path indexFilename; //output file name of index
bool overwrite_index; //overwrite index if it exists
bool create_index_only; //only create index and exit
bool split; //Split read mapping (done if this is true)
bool lower_triangular; // set to true if we should filter out half of the mappings
bool skip_self; //skip self mappings
bool skip_prefix; //skip mappings to sequences with the same prefix
bool overwrite_index = false; //overwrite index if it exists
bool create_index_only = false; //only create index and exit
bool split = false; //Split read mapping (done if this is true)
bool lower_triangular = false; // set to true if we should filter out half of the mappings
bool skip_self = false; //skip self mappings
bool skip_prefix = false; //skip mappings to sequences with the same prefix
char prefix_delim; //the prefix delimiter
std::string target_list; //file containing list of target sequences
std::string target_prefix; //prefix for target sequences to use
bool mergeMappings; //if we should merge consecutive segment mappings
bool keep_low_pct_id; //true if we should keep mappings whose estimated identity < percentageIdentity
bool report_ANI_percentage; //true if ANI should be in [0,100] as opposed to [0,1] (this is necessary for wfmash
bool filterLengthMismatches; //true if filtering out length mismatches
bool mergeMappings = false; //if we should merge consecutive segment mappings
bool keep_low_pct_id = false; //true if we should keep mappings whose estimated identity < percentageIdentity
bool report_ANI_percentage = false; //true if ANI should be in [0,100] as opposed to [0,1] (this is necessary for wfmash
bool filterLengthMismatches = false; //true if filtering out length mismatches
float kmerComplexityThreshold; //minimum kmer complexity to consider (default 0)

std::string query_list; // file containing list of query sequence names
Expand All @@ -74,19 +74,19 @@ struct Parameters
double hgNumerator = 1.0; // Numerator for the hypergeometric filter's Jaccard similarity calculation
uint64_t totalReferenceSize = 0; // Total size of all reference sequences
uint64_t estimatedUniqueKmers = 0; // Estimate of total unique k-mers
bool use_spaced_seeds; //
bool use_spaced_seeds = false; //
ales_params spaced_seed_params; //
double spaced_seed_sensitivity; //
std::vector<ales::spaced_seed> spaced_seeds; //
bool world_minimizers;
bool world_minimizers = false;
uint64_t sparsity_hash_threshold; // keep mappings that hash to <= this value
double overlap_threshold; // minimum overlap for a mapping to be considered
int64_t scaffold_max_deviation; // max diagonal deviation from scaffold chains
int64_t scaffold_gap; // gap threshold for scaffold chaining
int64_t scaffold_min_length = 50000; // minimum scaffold block length
std::string scaffold_output_file; // optional file to output scaffold mappings

bool legacy_output;
bool legacy_output = false;
//std::unordered_set<std::string> high_freq_kmers; //
int64_t index_by_size = std::numeric_limits<int64_t>::max(); // Target total size of sequences for each index subset
int minimum_hits = -1; // Minimum number of hits required for L1 filtering (-1 means auto)
Expand Down
20 changes: 16 additions & 4 deletions src/map/include/map_stats.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,17 +499,29 @@ namespace skch
}
}

// Thread-local reader map wrapper that ensures cleanup
struct ReaderMapWrapper {
std::map<std::string, faidx_reader_t*> readers;
~ReaderMapWrapper() {
for (auto& [filename, reader] : readers) {
if (reader) {
faidx_reader_destroy(reader);
}
}
}
};

// Function to get thread-local reader (per-file, per-thread)
auto getThreadLocalReader = [&](faidx_meta_t* meta, const std::string& filename) -> faidx_reader_t* {
thread_local std::map<std::string, faidx_reader_t*> readers;
thread_local ReaderMapWrapper reader_map;

auto it = readers.find(filename);
if (it == readers.end() || it->second == nullptr) {
auto it = reader_map.readers.find(filename);
if (it == reader_map.readers.end() || it->second == nullptr) {
faidx_reader_t* reader = faidx_reader_create(meta);
if (!reader) {
throw std::runtime_error("Failed to create thread-local reader for " + filename);
}
readers[filename] = reader;
reader_map.readers[filename] = reader;
return reader;
}
return it->second;
Expand Down