1From be41a98ac222f33ed5558d86e1cede67249e99b5 Mon Sep 17 00:00:00 2001 2From: Dmitry Vyukov <dvyukov@google.com> 3Date: Sat, 21 Mar 2020 13:34:50 +0100 4Subject: [PATCH] tsan: fix deadlock with pthread_atfork callbacks 5 6This fixes the bug reported at: 7https://groups.google.com/forum/#!topic/thread-sanitizer/e_zB9gYqFHM 8 9A pthread_atfork callback triggers a data race 10and we deadlock on the report_mtx. Ignore memory access 11in the pthread_atfork callbacks to prevent the deadlock. 12--- 13 compiler-rt/lib/tsan/rtl/tsan_rtl.cc | 9 ++++ 14 .../test/tsan/pthread_atfork_deadlock2.c | 49 +++++++++++++++++++ 15 2 files changed, 58 insertions(+) 16 create mode 100644 compiler-rt/test/tsan/pthread_atfork_deadlock2.c 17 18diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp 19index 3f3c0cce119..5e324a0a5fd 100644 20--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp 21+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp 22@@ -494,14 +494,23 @@ int Finalize(ThreadState *thr) { 23 void ForkBefore(ThreadState *thr, uptr pc) { 24 ctx->thread_registry->Lock(); 25 ctx->report_mtx.Lock(); 26+ // Ignore memory accesses in the pthread_atfork callbacks. 27+ // If any of them triggers a data race we will deadlock 28+ // on the report_mtx. 29+ // We could ignore interceptors and sync operations as well, 30+ // but so far it's unclear if it will do more good or harm. 31+ // Unnecessarily ignoring things can lead to false positives later. 32+ ThreadIgnoreBegin(thr, pc); 33 } 34 35 void ForkParentAfter(ThreadState *thr, uptr pc) { 36+ ThreadIgnoreEnd(thr, pc); // Begin is in ForkBefore. 37 ctx->report_mtx.Unlock(); 38 ctx->thread_registry->Unlock(); 39 } 40 41 void ForkChildAfter(ThreadState *thr, uptr pc) { 42+ ThreadIgnoreEnd(thr, pc); // Begin is in ForkBefore. 43 ctx->report_mtx.Unlock(); 44 ctx->thread_registry->Unlock(); 45 46diff --git a/compiler-rt/test/tsan/pthread_atfork_deadlock2.c b/compiler-rt/test/tsan/pthread_atfork_deadlock2.c 47new file mode 100644 48index 00000000000..700507c1e63 49--- /dev/null 50+++ b/compiler-rt/test/tsan/pthread_atfork_deadlock2.c 51@@ -0,0 +1,49 @@ 52+// RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s 53+// Regression test for 54+// https://groups.google.com/d/msg/thread-sanitizer/e_zB9gYqFHM/DmAiTsrLAwAJ 55+// pthread_atfork() callback triggers a data race and we deadlocked 56+// on the report_mtx as we lock it around fork. 57+#include "test.h" 58+#include <sys/types.h> 59+#include <sys/wait.h> 60+#include <errno.h> 61+ 62+int glob = 0; 63+ 64+void *worker(void *unused) { 65+ glob++; 66+ barrier_wait(&barrier); 67+ return NULL; 68+} 69+ 70+void atfork() { 71+ glob++; 72+} 73+ 74+int main() { 75+ barrier_init(&barrier, 2); 76+ pthread_atfork(atfork, NULL, NULL); 77+ pthread_t t; 78+ pthread_create(&t, NULL, worker, NULL); 79+ barrier_wait(&barrier); 80+ pid_t pid = fork(); 81+ if (pid < 0) { 82+ fprintf(stderr, "fork failed: %d\n", errno); 83+ return 1; 84+ } 85+ if (pid == 0) { 86+ fprintf(stderr, "CHILD\n"); 87+ return 0; 88+ } 89+ if (pid != waitpid(pid, NULL, 0)) { 90+ fprintf(stderr, "waitpid failed: %d\n", errno); 91+ return 1; 92+ } 93+ pthread_join(t, NULL); 94+ fprintf(stderr, "PARENT\n"); 95+ return 0; 96+} 97+ 98+// CHECK-NOT: ThreadSanitizer: data race 99+// CHECK: CHILD 100+// CHECK: PARENT 101