Data races in itimer_thread_func reported by ThreadSanitizer

Our local environment allows running Haskell programs under ThreadSanitizer. Even though the GHC runtime and compiled Haskell code are not instrumented, ThreadSanitizer still found a bug through interceptor instrumentation. I'll try to minimize the case and create a reproducer, but in case somebody wants to look at this with just the report, here it is (mildly massaged to remove irrelevant addresses):

==================
WARNING: ThreadSanitizer: data race (pid=2367)
  Write of size 1 at 0x55986a3e4f10 by thread T1:
    #0 pthread_mutex_destroy llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1245:3 
    #1 itimer_thread_func vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:152:5 

  Previous atomic read of size 1 at 0x55986a3e4f10 by main thread:
    #0 pthread_mutex_lock llvm/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:4097:3 
    #1 startTicker vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:184:5 
    #2 exitTicker vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:206 
    #3 __libc_start_main <null> (libc.so.6+0x38bbc)

  Location is global 'mutex' of size 40 at 0x55986a3e4f10 

  Thread T1 'ghc_ticker' (tid=2369, running) created by main thread at:
    #0 pthread_create llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:980:3 
    #1 initTicker vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:171:11 
    #2 __libc_start_main <null> (libc.so.6+0x38bbc)

SUMMARY: ThreadSanitizer: data race vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:152:5 in itimer_thread_func
==================
==================
WARNING: ThreadSanitizer: data race (pid=2367)
  Write of size 8 at 0x55986a3e4ee0 by thread T1:
    #0 pthread_cond_destroy llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1216:3 
    #1 itimer_thread_func vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:153:5 

  Previous read of size 8 at 0x55986a3e4ee0 by main thread (mutexes: write M146179626018688784):
    #0 pthread_cond_signal llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1202:3 
    #1 signalCondition vendor_src/v8_4_4/rts/posix/OSThreads.c:111:11 
    #2 __libc_start_main <null> (libc.so.6+0x38bbc)

  Location is global 'start_cond' of size 48 at 0x55986a3e4ee0 

  Mutex M146179626018688784 is already destroyed.

  Thread T1 'ghc_ticker' (tid=2369, running) created by main thread at:
    #0 pthread_create llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:980:3 
    #1 initTicker vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:171:11 
    #2 __libc_start_main <null> (libc.so.6+0x38bbc)

SUMMARY: ThreadSanitizer: data race vendor_src/v8_4_4/rts/posix/itimer/Pthread.c:153:5 in itimer_thread_func
==================
ThreadSanitizer: reported 2 warnings

Trac metadata

Trac field	Value
Version	8.4.4
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Runtime System
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	dvyukov@google.com
Operating system
Architecture

changed weight to 5

added RTS Tbug Trac import labels

Ahhh, I think we have actually seen this in manifest in practice in fact (in CI).

I believe this should be fixed by !95 (closed).

I believe #16147 (closed) is similar @bgamari, In that case there are multiple OS threads even on the non-threaded runtime. But the ACQUIRE_LOCK and RELEASE_LOCK macros expand to nothing on the non-threaded runtime, which introduces a race condition with shutdown of the loop and rts. Which is why I think it happens on the non-threaded but not the threaded.

See https://ghc.haskell.org/trac/ghc/attachment/ticket/16147/sample.txt

I think in this case we always want a lock, so ACQUIRE_LOCK and RELEASE_LOCK should be changed to OS_ACQUIRE_LOCK to OS_RELEASE_LOCK.

mentioned in commit 7b12b3f0

mentioned in commit ce11f6f2

changed milestone to %8.6.4

added 1 deleted label

closed

Merged with 4f180640 and 7ec385f4.

Trac metadata

Trac field	Value
Resolution	Unresolved → ResolvedFixed

removed 1 deleted label

added incorrect runtime result label

mentioned in commit 7ec385f4

mentioned in commit 4f180640

added Pnormal label

mentioned in issue #17090 (closed)