Merge "Revert "Unregister pthread_atfork handlers on dlclose()""

libc/Android.mk

@@ -63,7 +63,6 @@ libc_common_src_files := \
     stdio/sprintf.c \
     stdio/stdio.c \
     stdio/stdio_ext.cpp \
-    stdlib/atexit.c \
     stdlib/exit.c \
 
 # Fortify implementations of libc functions.
@@ -483,6 +482,7 @@ libc_upstream_openbsd_ndk_src_files := \
     upstream-openbsd/lib/libc/stdio/wprintf.c \
     upstream-openbsd/lib/libc/stdio/wscanf.c \
     upstream-openbsd/lib/libc/stdio/wsetup.c \
+    upstream-openbsd/lib/libc/stdlib/atexit.c \
     upstream-openbsd/lib/libc/stdlib/atoi.c \
     upstream-openbsd/lib/libc/stdlib/atol.c \
     upstream-openbsd/lib/libc/stdlib/atoll.c \

libc/arch-arm64/bionic/crtbegin.c

@@ -67,4 +67,3 @@ __asm__ (
 
 #include "../../arch-common/bionic/__dso_handle.h"
 #include "../../arch-common/bionic/atexit.h"
-#include "../../arch-common/bionic/pthread_atfork.h"

libc/arch-common/bionic/crtbegin.c

@@ -59,7 +59,6 @@ void _start() {
 
 #include "__dso_handle.h"
 #include "atexit.h"
-#include "pthread_atfork.h"
 #ifdef __i386__
 # include "../../arch-x86/bionic/__stack_chk_fail_local.h"
 #endif

libc/arch-common/bionic/crtbegin_so.c

@@ -56,7 +56,6 @@ void __on_dlclose() {
 # include "__dso_handle_so.h"
 # include "atexit.h"
 #endif
-#include "pthread_atfork.h"
 #ifdef __i386__
 # include "../../arch-x86/bionic/__stack_chk_fail_local.h"
 #endif

libc/arch-common/bionic/pthread_atfork.h

@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-extern void* __dso_handle;
-
-extern int __register_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void), void* dso);
-
-#ifndef _LIBC
-// Libc used to export this in previous versions, therefore it needs
-// to remain global for binary compatibility.
-__attribute__ ((visibility ("hidden")))
-#endif
-int pthread_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void)) {
-  return __register_atfork(prepare, parent, child, &__dso_handle);
-}
-

libc/arch-mips/bionic/crtbegin.c

@@ -92,4 +92,3 @@ __asm__ (
 
 #include "../../arch-common/bionic/__dso_handle.h"
 #include "../../arch-common/bionic/atexit.h"
-#include "../../arch-common/bionic/pthread_atfork.h"

libc/bionic/pthread_atfork.cpp

@@ -30,8 +30,6 @@
 #include <pthread.h>
 #include <stdlib.h>
 
-#include "private/bionic_macros.h"
-
 struct atfork_t {
   atfork_t* next;
   atfork_t* prev;
@@ -39,143 +37,79 @@ struct atfork_t {
   void (*prepare)(void);
   void (*child)(void);
   void (*parent)(void);
-
-  void* dso_handle;
 };
 
-class atfork_list_t {
+struct atfork_list_t {
- public:
+  atfork_t* first;
-  atfork_list_t() : first_(nullptr), last_(nullptr) {}
+  atfork_t* last;
-
-  template<typename F>
-  void walk_forward(F f) {
-    for (atfork_t* it = first_; it != nullptr; it = it->next) {
-      f(it);
-    }
-  }
-
-  template<typename F>
-  void walk_backwards(F f) {
-    for (atfork_t* it = last_; it != nullptr; it = it->prev) {
-      f(it);
-    }
-  }
-
-  void push_back(atfork_t* entry) {
-    entry->next = nullptr;
-    entry->prev = last_;
-    if (entry->prev != nullptr) {
-      entry->prev->next = entry;
-    }
-    if (first_ == nullptr) {
-      first_ = entry;
-    }
-    last_ = entry;
-  }
-
-  template<typename F>
-  void remove_if(F predicate) {
-    atfork_t* it = first_;
-    while (it != nullptr) {
-      if (predicate(it)) {
-        atfork_t* entry = it;
-        it = it->next;
-        remove(entry);
-      } else {
-        it = it->next;
-      }
-    }
-  }
-
- private:
-  void remove(atfork_t* entry) {
-    if (entry->prev != nullptr) {
-      entry->prev->next = entry->next;
-    } else {
-      first_ = entry->next;
-    }
-
-    if (entry->next != nullptr) {
-      entry->next->prev = entry->prev;
-    } else {
-      last_ = entry->prev;
-    }
-
-    free(entry);
-  }
-
-  atfork_t* first_;
-  atfork_t* last_;
-
-  DISALLOW_COPY_AND_ASSIGN(atfork_list_t);
 };
 
 static pthread_mutex_t g_atfork_list_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
-static atfork_list_t g_atfork_list;
+static atfork_list_t g_atfork_list = { NULL, NULL };
 
 void __bionic_atfork_run_prepare() {
   // We lock the atfork list here, unlock it in the parent, and reset it in the child.
   // This ensures that nobody can modify the handler array between the calls
   // to the prepare and parent/child handlers.
+  //
+  // TODO: If a handler tries to mutate the list, they'll block. We should probably copy
+  // the list before forking, and have prepare, parent, and child all work on the consistent copy.
   pthread_mutex_lock(&g_atfork_list_mutex);
 
   // Call pthread_atfork() prepare handlers. POSIX states that the prepare
   // handlers should be called in the reverse order of the parent/child
   // handlers, so we iterate backwards.
-  g_atfork_list.walk_backwards([](atfork_t* it) {
+  for (atfork_t* it = g_atfork_list.last; it != NULL; it = it->prev) {
-    if (it->prepare != nullptr) {
+    if (it->prepare != NULL) {
       it->prepare();
     }
-  });
+  }
 }
 
 void __bionic_atfork_run_child() {
-  g_atfork_list.walk_forward([](atfork_t* it) {
+  for (atfork_t* it = g_atfork_list.first; it != NULL; it = it->next) {
-    if (it->child != nullptr) {
+    if (it->child != NULL) {
       it->child();
     }
-  });
+  }
 
   g_atfork_list_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
 }
 
 void __bionic_atfork_run_parent() {
-  g_atfork_list.walk_forward([](atfork_t* it) {
+  for (atfork_t* it = g_atfork_list.first; it != NULL; it = it->next) {
-    if (it->parent != nullptr) {
+    if (it->parent != NULL) {
       it->parent();
     }
-  });
+  }
 
   pthread_mutex_unlock(&g_atfork_list_mutex);
 }
 
-// __register_atfork is the name used by glibc
+int pthread_atfork(void (*prepare)(void), void (*parent)(void), void(*child)(void)) {
-extern "C" int __register_atfork(void (*prepare)(void), void (*parent)(void),
-                                 void(*child)(void), void* dso) {
   atfork_t* entry = reinterpret_cast<atfork_t*>(malloc(sizeof(atfork_t)));
-  if (entry == nullptr) {
+  if (entry == NULL) {
     return ENOMEM;
   }
 
   entry->prepare = prepare;
   entry->parent = parent;
   entry->child = child;
-  entry->dso_handle = dso;
 
   pthread_mutex_lock(&g_atfork_list_mutex);
 
-  g_atfork_list.push_back(entry);
+  // Append 'entry' to the list.
+  entry->next = NULL;
+  entry->prev = g_atfork_list.last;
+  if (entry->prev != NULL) {
+    entry->prev->next = entry;
+  }
+  if (g_atfork_list.first == NULL) {
+    g_atfork_list.first = entry;
+  }
+  g_atfork_list.last = entry;
 
   pthread_mutex_unlock(&g_atfork_list_mutex);
 
   return 0;
 }
-
-extern "C" __LIBC_HIDDEN__ void __unregister_atfork(void* dso) {
-  pthread_mutex_lock(&g_atfork_list_mutex);
-  g_atfork_list.remove_if([&](const atfork_t* entry) {
-    return entry->dso_handle == dso;
-  });
-  pthread_mutex_unlock(&g_atfork_list_mutex);
-}
-

libc/upstream-openbsd/lib/libc/stdlib/atexit.c

@@ -35,15 +35,11 @@
 #include <string.h>
 #include <unistd.h>
 #include "atexit.h"
-#include "private/thread_private.h"
+#include "thread_private.h"
 
 struct atexit *__atexit;
 static int restartloop;
 
-/* BEGIN android-changed: __unregister_atfork is used by __cxa_finalize */
-extern void __unregister_atfork(void* dso);
-/* END android-changed */
-
 /*
  * Function pointers are stored in a linked list of pages. The list
  * is initially empty, and pages are allocated on demand. The first
@@ -66,7 +62,7 @@ __cxa_atexit(void (*func)(void *), void *arg, void *dso)
 {
 	struct atexit *p = __atexit;
 	struct atexit_fn *fnp;
-	size_t pgsize = getpagesize();
+	int pgsize = getpagesize();
 	int ret = -1;
 
 	if (pgsize < sizeof(*p))
@@ -165,12 +161,6 @@ restart:
 		__atexit = NULL;
 	}
 	_ATEXIT_UNLOCK();
-
-  /* BEGIN android-changed: call __unregister_atfork if dso is not null */
-  if (dso != NULL) {
-    __unregister_atfork(dso);
-  }
-  /* END android-changed */
 }
 
 /*
@@ -180,7 +170,7 @@ void
 __atexit_register_cleanup(void (*func)(void))
 {
 	struct atexit *p;
-	size_t pgsize = getpagesize();
+	int pgsize = getpagesize();
 
 	if (pgsize < sizeof(*p))
 		return;

tests/libs/Android.build.pthread_atfork.mk

@@ -1,25 +0,0 @@
-#
-# Copyright (C) 2014 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-# -----------------------------------------------------------------------------
-# This library used to test phtread_atfork handler behaviour
-# during/after dlclose.
-# -----------------------------------------------------------------------------
-libtest_pthread_atfork_src_files := pthread_atfork.cpp
-
-module := libtest_pthread_atfork
-include $(LOCAL_PATH)/Android.build.testlib.mk
-

tests/libs/Android.mk

@@ -25,7 +25,6 @@ common_additional_dependencies := \
     $(LOCAL_PATH)/Android.build.dlopen_check_order_dlsym.mk \
     $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_siblings.mk \
     $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_main_executable.mk \
-    $(LOCAL_PATH)/Android.build.pthread_atfork.mk \
     $(LOCAL_PATH)/Android.build.testlib.mk \
     $(LOCAL_PATH)/Android.build.versioned_lib.mk \
     $(TEST_PATH)/Android.build.mk
@@ -204,11 +203,6 @@ include $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_main_executable.mk
 # -----------------------------------------------------------------------------
 include $(LOCAL_PATH)/Android.build.versioned_lib.mk
 
-# -----------------------------------------------------------------------------
-# Build libraries needed by pthread_atfork tests
-# -----------------------------------------------------------------------------
-include $(LOCAL_PATH)/Android.build.pthread_atfork.mk
-
 # -----------------------------------------------------------------------------
 # Library with dependency loop used by dlfcn tests
 #

tests/libs/pthread_atfork.cpp

@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <pthread.h>
-
-extern "C" int proxy_pthread_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void)) {
-  return pthread_atfork(prepare, parent, child);
-}

tests/pthread_test.cpp

@@ -16,7 +16,6 @@
 
 #include <gtest/gtest.h>
 
-#include <dlfcn.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
@@ -988,14 +987,14 @@ TEST(pthread, pthread_once_1934122) {
 }
 
 static int g_atfork_prepare_calls = 0;
-static void AtForkPrepare1() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 1; }
+static void AtForkPrepare1() { g_atfork_prepare_calls = (g_atfork_prepare_calls << 4) | 1; }
-static void AtForkPrepare2() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 2; }
+static void AtForkPrepare2() { g_atfork_prepare_calls = (g_atfork_prepare_calls << 4) | 2; }
 static int g_atfork_parent_calls = 0;
-static void AtForkParent1() { g_atfork_parent_calls = (g_atfork_parent_calls * 10) + 1; }
+static void AtForkParent1() { g_atfork_parent_calls = (g_atfork_parent_calls << 4) | 1; }
-static void AtForkParent2() { g_atfork_parent_calls = (g_atfork_parent_calls * 10) + 2; }
+static void AtForkParent2() { g_atfork_parent_calls = (g_atfork_parent_calls << 4) | 2; }
 static int g_atfork_child_calls = 0;
-static void AtForkChild1() { g_atfork_child_calls = (g_atfork_child_calls * 10) + 1; }
+static void AtForkChild1() { g_atfork_child_calls = (g_atfork_child_calls << 4) | 1; }
-static void AtForkChild2() { g_atfork_child_calls = (g_atfork_child_calls * 10) + 2; }
+static void AtForkChild2() { g_atfork_child_calls = (g_atfork_child_calls << 4) | 2; }
 
 TEST(pthread, pthread_atfork_smoke) {
   ASSERT_EQ(0, pthread_atfork(AtForkPrepare1, AtForkParent1, AtForkChild1));
@@ -1006,71 +1005,13 @@ TEST(pthread, pthread_atfork_smoke) {
 
   // Child and parent calls are made in the order they were registered.
   if (pid == 0) {
-    ASSERT_EQ(12, g_atfork_child_calls);
+    ASSERT_EQ(0x12, g_atfork_child_calls);
     _exit(0);
   }
-  ASSERT_EQ(12, g_atfork_parent_calls);
+  ASSERT_EQ(0x12, g_atfork_parent_calls);
 
   // Prepare calls are made in the reverse order.
-  ASSERT_EQ(21, g_atfork_prepare_calls);
+  ASSERT_EQ(0x21, g_atfork_prepare_calls);
-  int status;
-  ASSERT_EQ(pid, waitpid(pid, &status, 0));
-}
-
-static void AtForkPrepare3() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 3; }
-static void AtForkPrepare4() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 4; }
-
-static void AtForkParent3() { g_atfork_parent_calls = (g_atfork_parent_calls * 10) + 3; }
-static void AtForkParent4() { g_atfork_parent_calls = (g_atfork_parent_calls * 10) + 4; }
-
-static void AtForkChild3() { g_atfork_child_calls = (g_atfork_child_calls * 10) + 3; }
-static void AtForkChild4() { g_atfork_child_calls = (g_atfork_child_calls * 10) + 4; }
-
-TEST(pthread, pthread_atfork_with_dlclose) {
-  ASSERT_EQ(0, pthread_atfork(AtForkPrepare1, AtForkParent1, AtForkChild1));
-
-  void* handle = dlopen("libtest_pthread_atfork.so", RTLD_NOW | RTLD_LOCAL);
-  ASSERT_TRUE(handle != nullptr) << dlerror();
-  typedef int (*fn_t)(void (*)(void), void (*)(void), void (*)(void));
-  fn_t fn = reinterpret_cast<fn_t>(dlsym(handle, "proxy_pthread_atfork"));
-  ASSERT_TRUE(fn != nullptr) << dlerror();
-  // the library registers 2 additional atfork handlers in a constructor
-  ASSERT_EQ(0, fn(AtForkPrepare2, AtForkParent2, AtForkChild2));
-  ASSERT_EQ(0, fn(AtForkPrepare3, AtForkParent3, AtForkChild3));
-
-  ASSERT_EQ(0, pthread_atfork(AtForkPrepare4, AtForkParent4, AtForkChild4));
-
-  int pid = fork();
-
-  ASSERT_NE(-1, pid) << strerror(errno);
-
-  if (pid == 0) {
-    ASSERT_EQ(1234, g_atfork_child_calls);
-    _exit(0);
-  }
-
-  ASSERT_EQ(1234, g_atfork_parent_calls);
-  ASSERT_EQ(4321, g_atfork_prepare_calls);
-
-  EXPECT_EQ(0, dlclose(handle));
-  g_atfork_prepare_calls = g_atfork_parent_calls = g_atfork_child_calls = 0;
-
-  int status;
-  ASSERT_EQ(pid, waitpid(pid, &status, 0));
-
-  pid = fork();
-
-  ASSERT_NE(-1, pid) << strerror(errno);
-
-  if (pid == 0) {
-    ASSERT_EQ(14, g_atfork_child_calls);
-    _exit(0);
-  }
-
-  ASSERT_EQ(14, g_atfork_parent_calls);
-  ASSERT_EQ(41, g_atfork_prepare_calls);
-
-  ASSERT_EQ(pid, waitpid(pid, &status, 0));
 }
 
 TEST(pthread, pthread_attr_getscope) {