Revert "Unregister pthread_atfork handlers on dlclose()"
The visibility control in pthread_atfork.h is incorrect.
It breaks 64bit libc.so by hiding pthread_atfork.
This reverts commit 6df122f8528f9b9fcf7dfea14ae98b0ef66274e1.
Change-Id: I21e4b344d500c6f6de0ccb7420b916c4e233dd34
diff --git a/libc/Android.mk b/libc/Android.mk
index 8ed969f..4a199e7 100644
--- a/libc/Android.mk
+++ b/libc/Android.mk
@@ -63,7 +63,6 @@
stdio/sprintf.c \
stdio/stdio.c \
stdio/stdio_ext.cpp \
- stdlib/atexit.c \
stdlib/exit.c \
# Fortify implementations of libc functions.
@@ -482,6 +481,7 @@
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 \
diff --git a/libc/arch-arm64/bionic/crtbegin.c b/libc/arch-arm64/bionic/crtbegin.c
index 7e2c5d7..fec0b11 100644
--- a/libc/arch-arm64/bionic/crtbegin.c
+++ b/libc/arch-arm64/bionic/crtbegin.c
@@ -67,4 +67,3 @@
#include "../../arch-common/bionic/__dso_handle.h"
#include "../../arch-common/bionic/atexit.h"
-#include "../../arch-common/bionic/pthread_atfork.h"
diff --git a/libc/arch-common/bionic/crtbegin.c b/libc/arch-common/bionic/crtbegin.c
index c46405c..fa9f3f3 100644
--- a/libc/arch-common/bionic/crtbegin.c
+++ b/libc/arch-common/bionic/crtbegin.c
@@ -59,7 +59,6 @@
#include "__dso_handle.h"
#include "atexit.h"
-#include "pthread_atfork.h"
#ifdef __i386__
# include "../../arch-x86/bionic/__stack_chk_fail_local.h"
#endif
diff --git a/libc/arch-common/bionic/crtbegin_so.c b/libc/arch-common/bionic/crtbegin_so.c
index 3754363..641e45a 100644
--- a/libc/arch-common/bionic/crtbegin_so.c
+++ b/libc/arch-common/bionic/crtbegin_so.c
@@ -56,7 +56,6 @@
# 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
diff --git a/libc/arch-common/bionic/pthread_atfork.h b/libc/arch-common/bionic/pthread_atfork.h
deleted file mode 100644
index 0c48a12..0000000
--- a/libc/arch-common/bionic/pthread_atfork.h
+++ /dev/null
@@ -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);
-}
-
diff --git a/libc/arch-mips/bionic/crtbegin.c b/libc/arch-mips/bionic/crtbegin.c
index d72ec7b..50e9eeb 100644
--- a/libc/arch-mips/bionic/crtbegin.c
+++ b/libc/arch-mips/bionic/crtbegin.c
@@ -92,4 +92,3 @@
#include "../../arch-common/bionic/__dso_handle.h"
#include "../../arch-common/bionic/atexit.h"
-#include "../../arch-common/bionic/pthread_atfork.h"
diff --git a/libc/bionic/pthread_atfork.cpp b/libc/bionic/pthread_atfork.cpp
index 093ffd2..d1c4ad0 100644
--- a/libc/bionic/pthread_atfork.cpp
+++ b/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 @@
void (*prepare)(void);
void (*child)(void);
void (*parent)(void);
-
- void* dso_handle;
};
-class atfork_list_t {
- public:
- atfork_list_t() : first_(nullptr), last_(nullptr) {}
-
- 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);
+struct atfork_list_t {
+ atfork_t* first;
+ atfork_t* last;
};
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) {
- if (it->prepare != nullptr) {
+ for (atfork_t* it = g_atfork_list.last; it != NULL; it = it->prev) {
+ if (it->prepare != NULL) {
it->prepare();
}
- });
+ }
}
void __bionic_atfork_run_child() {
- g_atfork_list.walk_forward([](atfork_t* it) {
- if (it->child != nullptr) {
+ for (atfork_t* it = g_atfork_list.first; it != NULL; it = it->next) {
+ 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) {
- if (it->parent != nullptr) {
+ for (atfork_t* it = g_atfork_list.first; it != NULL; it = it->next) {
+ if (it->parent != NULL) {
it->parent();
}
- });
+ }
pthread_mutex_unlock(&g_atfork_list_mutex);
}
-// __register_atfork is the name used by glibc
-extern "C" int __register_atfork(void (*prepare)(void), void (*parent)(void),
- void(*child)(void), void* dso) {
+int pthread_atfork(void (*prepare)(void), void (*parent)(void), void(*child)(void)) {
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);
-}
-
diff --git a/libc/stdlib/atexit.c b/libc/upstream-openbsd/lib/libc/stdlib/atexit.c
similarity index 92%
rename from libc/stdlib/atexit.c
rename to libc/upstream-openbsd/lib/libc/stdlib/atexit.c
index df2b1b5..6532b38 100644
--- a/libc/stdlib/atexit.c
+++ b/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 @@
{
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 @@
__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 @@
__atexit_register_cleanup(void (*func)(void))
{
struct atexit *p;
- size_t pgsize = getpagesize();
+ int pgsize = getpagesize();
if (pgsize < sizeof(*p))
return;
diff --git a/libc/stdlib/atexit.h b/libc/upstream-openbsd/lib/libc/stdlib/atexit.h
similarity index 100%
rename from libc/stdlib/atexit.h
rename to libc/upstream-openbsd/lib/libc/stdlib/atexit.h
diff --git a/tests/libs/Android.build.pthread_atfork.mk b/tests/libs/Android.build.pthread_atfork.mk
deleted file mode 100644
index 72ffec4..0000000
--- a/tests/libs/Android.build.pthread_atfork.mk
+++ /dev/null
@@ -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
-
diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk
index c78661e..3d5b060 100644
--- a/tests/libs/Android.mk
+++ b/tests/libs/Android.mk
@@ -25,7 +25,6 @@
$(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
@@ -205,11 +204,6 @@
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
#
# libtest_with_dependency_loop -> a -> b -> c -> a
diff --git a/tests/libs/pthread_atfork.cpp b/tests/libs/pthread_atfork.cpp
deleted file mode 100644
index 3a5aa4f..0000000
--- a/tests/libs/pthread_atfork.cpp
+++ /dev/null
@@ -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);
-}
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index b8cfd56..a299f02 100644
--- a/tests/pthread_test.cpp
+++ b/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 @@
}
static int g_atfork_prepare_calls = 0;
-static void AtForkPrepare1() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 1; }
-static void AtForkPrepare2() { g_atfork_prepare_calls = (g_atfork_prepare_calls * 10) + 2; }
+static void AtForkPrepare1() { g_atfork_prepare_calls = (g_atfork_prepare_calls << 4) | 1; }
+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 AtForkParent2() { g_atfork_parent_calls = (g_atfork_parent_calls * 10) + 2; }
+static void AtForkParent1() { g_atfork_parent_calls = (g_atfork_parent_calls << 4) | 1; }
+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 AtForkChild2() { g_atfork_child_calls = (g_atfork_child_calls * 10) + 2; }
+static void AtForkChild1() { g_atfork_child_calls = (g_atfork_child_calls << 4) | 1; }
+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 @@
// 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);
- 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));
+ ASSERT_EQ(0x21, g_atfork_prepare_calls);
}
TEST(pthread, pthread_attr_getscope) {
