Ramki-Ravindran / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

atexit() doesn't work if called after forking a multithreaded program with die_after_fork=[01] #57

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
$ cat t.cc 
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

void foo() {
  printf("bye!\n");
}

void *worker(void *unused) {
  return NULL;
}

int main() {
  pthread_t t;
  pthread_create(&t, NULL, worker, NULL);
  if (fork() == 0) {
    atexit(foo);
  }
  return 0;
}

$ bin/clang -fsanitize=thread t.cc -o t -g
$ ./t
==9217==FATAL: ThreadSanitizer: unreachable called

Original issue reported on code.google.com by gli...@chromium.org on 28 Mar 2014 at 2:45

GoogleCodeExporter commented 9 years ago
According to debug printing, REAL(atexit) is never initialized with 
dlsym("atexit") (not sure if it needs to).
Normally INTERCEPTOR(atexit) never calls REAL(atexit), except for the following 
case:

(tsan_interceptors.cc)
 204 #define SCOPED_TSAN_INTERCEPTOR(func, ...) \
 ...
 210     if (thr->ignore_interceptors || thr->in_ignored_lib) \
 211       return REAL(func)(__VA_ARGS__); \

But when detecting the multithreaded fork() we're ignoring all the interceptors 
regardless of the die_after_fork value:
(tsan_rtl.cc)
336 void ForkChildAfter(ThreadState *thr, uptr pc) {
337   ctx->report_mtx.Unlock();
338   ctx->thread_registry->Unlock();
339 
340   uptr nthread = 0;
341   ctx->thread_registry->GetNumberOfThreads(0, 0, &nthread /* alive threads 
*/);
342   VPrintf(1, "ThreadSanitizer: forked new process with pid %d,"
343       " parent had %d threads\n", (int)internal_getpid(), (int)nthread);
344   if (nthread == 1) {
345     internal_start_thread(&BackgroundThread, 0);
346   } else {
347     // We've just forked a multi-threaded process. We cannot reasonably 
function
348     // after that (some mutexes may be locked before fork). So just enable
349     // ignores for everything in the hope that we will exec soon.
350     ctx->after_multithreaded_fork = true;
351     thr->ignore_interceptors++;

(tsan_rtl_thread.cc)
230 void ThreadStart(ThreadState *thr, int tid, uptr os_id) {
...
264 #ifndef TSAN_GO
265   if (ctx->after_multithreaded_fork) {
266     thr->ignore_interceptors++;
267     ThreadIgnoreBegin(thr, 0);
268     ThreadIgnoreSyncBegin(thr, 0);
269   }
270 #endif
271 }

Original comment by gli...@google.com on 28 Mar 2014 at 2:57

GoogleCodeExporter commented 9 years ago
I see two possible ways to fix this:
1) Turn ignore_interceptors on only if die_after_fork is 1.
This is arguable, we may call an intercepted function before exec(), which may 
deadlock.
2) Initialize REAL(atexit) with something more sane (reimplement it without 
locks?)

We should anyway turn ignore_interceptors off when the user's intent to run 
further is evident (e.g. when the user spawns another thread).

Original comment by gli...@google.com on 28 Mar 2014 at 3:11

GoogleCodeExporter commented 9 years ago

Original comment by gli...@chromium.org on 28 Mar 2014 at 3:20

GoogleCodeExporter commented 9 years ago
While at this, I think we need to make the ignore stuff more 
visible/debuggable. IIUC the user who runs with die_after_fork=0 may easily 
miss all the races which happen after an incorrect fork().

Original comment by gli...@google.com on 28 Mar 2014 at 4:17

GoogleCodeExporter commented 9 years ago
Fixed by r206980.

Original comment by dvyu...@google.com on 23 Apr 2014 at 1:51