Skip to content

Commit

Permalink
[FIX] make a bgzf_thread_count a single variable
Browse files Browse the repository at this point in the history
bgzf_thread_count was declared `inline static` outside of the scope of a
class.
In this case the `inline` allows the variable to break the one
definition rule. Meaning there can be multiple symbols across different
translation that all declare `bgzf_thread_count`. In this case the
linker will choose one of the defined symbols. (Without inline it would
throw a multiple definition error).

The `static` keyword (in this context) means that the created symbol is
only visible inside a current translation unit. Meaning, there are no symbols
for the linker to work with. This will cause every translation unit to have there
own `bgzf_thread_count` variable.

This combination leads to every translation unit having there own
`bgzf_thread_count` variable, which is not what a user of seqan3 expects.
The fix is simple, we remove the `static` keyword. This should have the
intended behavior.

Thought: maybe it should be declared `thread_local`.
  • Loading branch information
SGSSGene committed Nov 30, 2021
1 parent b3588d1 commit 646a685
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion include/seqan3/contrib/stream/bgzf_stream_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace seqan3::contrib
/*!\brief A static variable indicating the number of threads to use for the bgzf-streams.
* Defaults to std::thread::hardware_concurrency.
*/
inline static uint64_t bgzf_thread_count = std::thread::hardware_concurrency();
inline uint64_t bgzf_thread_count = std::thread::hardware_concurrency();

// ============================================================================
// Forwards
Expand Down

0 comments on commit 646a685

Please sign in to comment.