samtools / htslib

C library for high-throughput sequencing data formats
Other
784 stars 447 forks source link

Always do the CRAM mutex lock/destroys. #1681

Closed jkbonfield closed 9 months ago

jkbonfield commented 9 months ago

For simplicity and to remove a lot of conditionals the code using mutexes is always used irrespective of whether we are multi-threading.

For example cram_get_ref starts with:

pthread_mutex_lock(&fd->ref_lock);

The mutexes themselves however are only initialised when using cram_set_voption with CRAM_OPT_NTHREADS or CRAM_OPT_THREAD_POOL option. Attempting to lock an uninitialised mutex has undefined behaviour.

On linux this does nothing as A) we're not checking the return value from the lock functions anyway (the only meaningful error is not-initialised) and B) we calloc the structure and a blank mutex is the basically the same as initialising it.

Under MSVC however this leads to crashes. So moved both init and destroy into the cram_dopen function. Note this adds an additional destroy which was previously forgotten (fd->range_lock), but again under linux there is no memory allocated so lacking a destroy will not leak resources.

Reported by Sebastian Deorowicz