Workaround attempt for timeout flakiness: Avoid running atexit() handlers in fork()ed process on li…

Workaround attempt for timeout flakiness: Avoid running atexit() handlers in fork()ed process on linux

About this CL:

  The only purpose of `if (fork() == 0) exit(0)` is to wake up
  a thread in the parent process which might be blocked on `wait()`.

  There is no need to run atexit() handlers in the `fork()`ed child.

  This is a *workaround attempt* for a deadlocked `free()` call inside the
  processing of atexit handlers in glibc.

  (Side note: There might be better ways of notifying the thread, like sending a
   signal to the particular pthread with `pthread_kill` which would make the
   `wait()` syscall be interrupted.)

About the issue:

  It is still unclear why, in this particular case, the tcmalloc locks should
  be hold during the `exit()` call:

    * via a static initializer tcmalloc uses
        `pthread_atfork(before=ObtainAllLocks(),
                        after_parent=ReleaseAllLocks(),
                        after_child=ReleaseAllLocks())`
      to register locking & unlocking around `fork()`

    * glibc's `fork()` runs the either `after_parent` or `after_child` handlers
      (unconditionally) which should free the locks

    * the `exit()` call later should be free to malloc/free

  The [BUG] describes more in detail how we can hit a tcmalloc deadlock in a
  different situation (it's a linux kernel bug).
  Namely, if the linux kernel runs OOM during `fork()` and therefore fails to
  set the new thread-id. The glibc code hits an assert and tries to allocate
  memory before `after_parent`/`after_child` handlers were executed which
  deadlocks.

BUG=https://github.com/dart-lang/sdk/issues/28246
R=vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/e0a427a9572c41804b7f45008a6725fe376b17f9

[kustermann, 2017-01-05 12:01:05]

kustermann@google.com changed reviewers:
+ vegorov@google.com

[kustermann, 2017-01-05 12:01:05]

I could also try to get rid of the `if (fork() == 0) exit(0)` in this case
entirely by using pthread_kill(), but there might be interactions with signals
(because they are also handled specially in the VM).

[Vyacheslav Egorov (Google), 2017-01-05 12:43:37]

lgtm

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_android.cc
File runtime/bin/process_android.cc (right):

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_android...
runtime/bin/process_android.cc:168: _exit(0);
Please leave a comment here why _exit and not exit.

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_linux.cc
File runtime/bin/process_linux.cc (right):

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_linux.c...
runtime/bin/process_linux.cc:168: _exit(0);
ditto

[kustermann, 2017-01-05 12:55:34]

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_android.cc
File runtime/bin/process_android.cc (right):

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_android...
runtime/bin/process_android.cc:168: _exit(0);
On 2017/01/05 12:43:37, Vyacheslav Egorov (Google) wrote:
> Please leave a comment here why _exit and not exit.

Done.

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_linux.cc
File runtime/bin/process_linux.cc (right):

https://codereview.chromium.org/2618723002/diff/1/runtime/bin/process_linux.c...
runtime/bin/process_linux.cc:168: _exit(0);
On 2017/01/05 12:43:37, Vyacheslav Egorov (Google) wrote:
> ditto

Done.

[kustermann, 2017-01-05 12:56:09]

Description was changed from

==========
Workaround attempt for timeout flakiness: Avoid running atexit() handlers in
fork()ed process on linux

About this CL:

  The only purpose of `if (fork() == 0) exit(0)` is to wake up
  a thread in the parent process which might be blocked on `wait()`.

  There is no need to run atexit() handlers in the `fork()`ed child.

  This is a *workaround attempt* for a deadlocked `free()` call inside the
  processing of atexit handlers in glibc.

  (Side note: There might be better ways of notifying the thread, like sending a
   signal to the particular pthread with `pthread_kill` which would make the
   `wait()` syscall be interrupted.)

About the issue:

  It is still unclear why, in this particular case, the tcmalloc locks should
  be hold during the `exit()` call:

    * via a static initializer tcmalloc uses
        `pthread_atfork(before=ObtainAllLocks(),
                        after_parent=ReleaseAllLocks(),
                        after_child=ReleaseAllLocks())`
      to register locking & unlocking around `fork()`

    * glibc's `fork()` runs the either `after_parent` or `after_child` handlers
      (unconditionally) which should free the locks

    * the `exit()` call later should be free to malloc/free

  The [BUG] describes more in detail how we can hit a tcmalloc deadlock in a
  different situation (it's a linux kernel bug).
  Namely, if the linux kernel runs OOM during `fork()` and therefore fails to
  set the new thread-id. The glibc code hits an assert and tries to allocate
  memory before `after_parent`/`after_child` handlers were executed which
  deadlocks.

BUG=https://github.com/dart-lang/sdk/issues/28246
==========

to

==========
Workaround attempt for timeout flakiness: Avoid running atexit() handlers in
fork()ed process on linux

About this CL:

  The only purpose of `if (fork() == 0) exit(0)` is to wake up
  a thread in the parent process which might be blocked on `wait()`.

  There is no need to run atexit() handlers in the `fork()`ed child.

  This is a *workaround attempt* for a deadlocked `free()` call inside the
  processing of atexit handlers in glibc.

  (Side note: There might be better ways of notifying the thread, like sending a
   signal to the particular pthread with `pthread_kill` which would make the
   `wait()` syscall be interrupted.)

About the issue:

  It is still unclear why, in this particular case, the tcmalloc locks should
  be hold during the `exit()` call:

    * via a static initializer tcmalloc uses
        `pthread_atfork(before=ObtainAllLocks(),
                        after_parent=ReleaseAllLocks(),
                        after_child=ReleaseAllLocks())`
      to register locking & unlocking around `fork()`

    * glibc's `fork()` runs the either `after_parent` or `after_child` handlers
      (unconditionally) which should free the locks

    * the `exit()` call later should be free to malloc/free

  The [BUG] describes more in detail how we can hit a tcmalloc deadlock in a
  different situation (it's a linux kernel bug).
  Namely, if the linux kernel runs OOM during `fork()` and therefore fails to
  set the new thread-id. The glibc code hits an assert and tries to allocate
  memory before `after_parent`/`after_child` handlers were executed which
  deadlocks.

BUG=https://github.com/dart-lang/sdk/issues/28246
R=vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e0a427a9572c41804b7f45008a6725fe376b17f9
==========

[kustermann, 2017-01-05 12:56:10]

Committed patchset #2 (id:20001) manually as
e0a427a9572c41804b7f45008a6725fe376b17f9 (presubmit successful).