A data race in nhmmer v3.3

[Augustin-Zidek]

I ran Thread Sanitizer on our nhmmer test and it found this issue:

==================
WARNING: ThreadSanitizer: data race (pid=7289)
  Write of size 8 at 0x7b50000070f0 by main thread:
    #0 thread_loop hmmer/src/nhmmer.c:1447:24
    #1 serial_master hmmer/src/nhmmer.c:1035:35
    #2 main hmmer/src/nhmmer.c:445:12

  Previous read of size 8 at 0x7b50000070f0 by thread T17:
    #0 p7_pli_NewSeq hmmer/src/p7_pipeline.c:580:88
    #1 pipeline_thread hmmer/src/nhmmer.c:1562:7

  Location is heap block of size 512 at 0x7b5000007000 allocated by main thread:
    #0 calloc llvm/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:687:5
    #1 p7_pipeline_Create hmmer/src/p7_pipeline.c:105:3
    #2 serial_master hmmer/src/nhmmer.c:977:25
    #3 main hmmer/src/nhmmer.c:445:12

  Thread T17 (tid=7307, running) created by main thread at:
    #0 pthread_create llvm/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1022:3
    #1 esl_threads_AddThread hmmer/easel/esl_threads.c:128:7
    #2 serial_master hmmer/src/nhmmer.c:1016:13
    #3 main hmmer/src/nhmmer.c:445:12

SUMMARY: ThreadSanitizer: data race hmmer/src/nhmmer.c:1447:24 in thread_loop

This is with hmmer v3.3 downloaded on 2020-04-07, the problematic line in nhmmer is the following:

info->pli->nseqs += block->count  - ((abort || block->complete) ? 0 : 1);// if there's an incomplete sequence read into the block wait to count it until it's complete.

This issue can be fixed by making nseqs atomic in the P7_PIPELINE struct in hmmer.h, i.e. changing:

uint64_t      nseqs;            /* # of sequences searched                  */

to

#include <stdatomic.h>
// ...
atomic_size_t      nseqs;           /* # of sequences searched                  */

I am however unsure whether this is a proper fix, or whether it indicates there is a more serious underlying issue. Could you advise whether this fix is correct?

[cryptogenomicon]

I've committed two fixes to HMMER and one to its underlying Easel library that fix this and other tsan failures. The fixes will appear in the next HMMER3 release.

Your fix also works but requires C11 compliance to get the types, and we're not ready to move off a minimum of C99 compliance yet.

The problems are cosmetic only - I believe all results are still correct despite the races, and the fixes do not have any impact on HMMER results, only on tsan and valgrind/helgrind problem detection.

Thank you!

[Augustin-Zidek]

Thanks for fixing. Looking forward to the next release.