Merge "Add secondary counter in place of tls_nodelete flag"

linker/linker.cpp

@@ -1936,7 +1936,7 @@ void increment_dso_handle_reference_counter(void* dso_handle) {
     soinfo* si = find_containing_library(dso_handle);
     if (si != nullptr) {
       ProtectedDataGuard guard;
-      si->set_tls_nodelete();
+      si->increment_ref_count();
     } else {
       async_safe_fatal(
           "increment_dso_handle_reference_counter: Couldn't find soinfo by dso_handle=%p",
@@ -1959,11 +1959,7 @@ void decrement_dso_handle_reference_counter(void* dso_handle) {
     soinfo* si = find_containing_library(dso_handle);
     if (si != nullptr) {
       ProtectedDataGuard guard;
-      si->unset_tls_nodelete();
-      if (si->get_ref_count() == 0) {
-        // Perform deferred unload - note that soinfo_unload_impl does not decrement ref_count
-        soinfo_unload_impl(si);
-      }
+      soinfo_unload(si);
     } else {
       async_safe_fatal(
           "decrement_dso_handle_reference_counter: Couldn't find soinfo by dso_handle=%p",

linker/linker_soinfo.cpp

@@ -537,14 +537,6 @@ void soinfo::set_nodelete() {
   rtld_flags_ |= RTLD_NODELETE;
 }
 
-void soinfo::set_tls_nodelete() {
-  flags_ |= FLAG_TLS_NODELETE;
-}
-
-void soinfo::unset_tls_nodelete() {
-  flags_ &= ~FLAG_TLS_NODELETE;
-}
-
 const char* soinfo::get_realpath() const {
 #if defined(__work_around_b_24465209__)
   if (has_min_version(2)) {
@@ -660,8 +652,7 @@ bool soinfo::is_gnu_hash() const {
 bool soinfo::can_unload() const {
   return !is_linked() ||
          (
-             (get_rtld_flags() & (RTLD_NODELETE | RTLD_GLOBAL)) == 0 &&
-             (flags_ & FLAG_TLS_NODELETE) == 0
+             (get_rtld_flags() & (RTLD_NODELETE | RTLD_GLOBAL)) == 0
          );
 }
 

linker/linker_soinfo.h

@@ -51,7 +51,7 @@
                                          // when load group is crossing
                                          // namespace boundary twice and second
                                          // local group depends on the same libraries.
-#define FLAG_TLS_NODELETE     0x00000200 // This flag set when there is at least one
+#define FLAG_RESERVED         0x00000200 // This flag was set when there is at least one
                                          // outstanding thread_local dtor
                                          // registered with this soinfo. In such
                                          // a case the actual unload is
@@ -260,8 +260,6 @@ struct soinfo {
   void set_linker_flag();
   void set_main_executable();
   void set_nodelete();
-  void set_tls_nodelete();
-  void unset_tls_nodelete();
 
   size_t increment_ref_count();
   size_t decrement_ref_count();

tests/Android.bp

@@ -573,6 +573,7 @@ cc_test {
         "libtest_with_dependency_loop_c",
         "libtest_with_dependency_loop",
         "libtest_with_dependency",
+        "libtest_indirect_thread_local_dtor",
         "libtest_invalid-empty_shdr_table.so",
         "libtest_invalid-rw_load_segment.so",
         "libtest_invalid-unaligned_shdr_offset.so",
@@ -583,6 +584,7 @@ cc_test {
         "libtest_invalid-textrels.so",
         "libtest_invalid-textrels2.so",
         "libtest_thread_local_dtor",
+        "libtest_thread_local_dtor2",
         "preinit_getauxval_test_helper",
         "preinit_syscall_test_helper",
         "libnstest_private_external",

tests/dlfcn_test.cpp

@@ -1329,7 +1329,7 @@ TEST(dlfcn, dt_runpath_absolute_path) {
   dlclose(handle);
 }
 
-TEST(dlfcn, dlclose_after_thread_local_dtor) {
+static void test_dlclose_after_thread_local_dtor(const char* library_name) {
   bool is_dtor_triggered = false;
 
   auto f = [](void* handle, bool* is_dtor_triggered) {
@@ -1342,10 +1342,10 @@ TEST(dlfcn, dlclose_after_thread_local_dtor) {
     ASSERT_TRUE(!*is_dtor_triggered);
   };
 
-  void* handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  void* handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
   ASSERT_TRUE(handle == nullptr);
 
-  handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW);
+  handle = dlopen(library_name, RTLD_NOW);
   ASSERT_TRUE(handle != nullptr) << dlerror();
 
   std::thread t(f, handle, &is_dtor_triggered);
@@ -1354,18 +1354,26 @@ TEST(dlfcn, dlclose_after_thread_local_dtor) {
   ASSERT_TRUE(is_dtor_triggered);
   dlclose(handle);
 
-  handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
   ASSERT_TRUE(handle == nullptr);
 }
 
-TEST(dlfcn, dlclose_before_thread_local_dtor) {
+TEST(dlfcn, dlclose_after_thread_local_dtor) {
+  test_dlclose_after_thread_local_dtor("libtest_thread_local_dtor.so");
+}
+
+TEST(dlfcn, dlclose_after_thread_local_dtor_indirect) {
+  test_dlclose_after_thread_local_dtor("libtest_indirect_thread_local_dtor.so");
+}
+
+static void test_dlclose_before_thread_local_dtor(const char* library_name) {
   bool is_dtor_triggered = false;
 
-  auto f = [](bool* is_dtor_triggered) {
-    void* handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  auto f = [library_name](bool* is_dtor_triggered) {
+    void* handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
     ASSERT_TRUE(handle == nullptr);
 
-    handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW);
+    handle = dlopen(library_name, RTLD_NOW);
     ASSERT_TRUE(handle != nullptr) << dlerror();
 
     typedef void (*fn_t)(bool*);
@@ -1380,16 +1388,16 @@ TEST(dlfcn, dlclose_before_thread_local_dtor) {
 
     // Since we have thread_atexit dtors associated with handle - the library should
     // still be availabe.
-    handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+    handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
     ASSERT_TRUE(handle != nullptr) << dlerror();
     dlclose(handle);
   };
 
-  void* handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW);
+  void* handle = dlopen(library_name, RTLD_NOW);
   ASSERT_TRUE(handle != nullptr) << dlerror();
   dlclose(handle);
 
-  handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
   ASSERT_TRUE(handle == nullptr);
 
   std::thread t(f, &is_dtor_triggered);
@@ -1397,12 +1405,126 @@ TEST(dlfcn, dlclose_before_thread_local_dtor) {
 #if defined(__BIONIC__)
   // ld-android.so unloads unreferenced libraries on pthread_exit()
   ASSERT_TRUE(is_dtor_triggered);
-  handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
   ASSERT_TRUE(handle == nullptr);
 #else
   // GLIBC does not unload libraries with ref_count = 0 on pthread_exit
   ASSERT_TRUE(is_dtor_triggered);
-  handle = dlopen("libtest_thread_local_dtor.so", RTLD_NOW | RTLD_NOLOAD);
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+  ASSERT_TRUE(handle != nullptr) << dlerror();
+#endif
+}
+
+TEST(dlfcn, dlclose_before_thread_local_dtor) {
+  test_dlclose_before_thread_local_dtor("libtest_thread_local_dtor.so");
+}
+
+TEST(dlfcn, dlclose_before_thread_local_dtor_indirect) {
+  test_dlclose_before_thread_local_dtor("libtest_indirect_thread_local_dtor.so");
+}
+
+TEST(dlfcn, dlclose_before_thread_local_dtor_multiple_dsos) {
+  const constexpr char* library_name = "libtest_indirect_thread_local_dtor.so";
+
+  bool is_dtor1_triggered = false;
+  bool is_dtor2_triggered = false;
+
+  std::mutex mtx;
+  std::condition_variable cv;
+  void* library_handle = nullptr;
+  bool thread1_dlopen_complete = false;
+  bool thread2_thread_local_dtor_initialized = false;
+  bool thread1_complete = false;
+
+  auto f1 = [&]() {
+    void* handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+    ASSERT_TRUE(handle == nullptr);
+
+    handle = dlopen(library_name, RTLD_NOW);
+    ASSERT_TRUE(handle != nullptr) << dlerror();
+    std::unique_lock<std::mutex> lock(mtx);
+    thread1_dlopen_complete = true;
+    library_handle = handle;
+    lock.unlock();
+    cv.notify_one();
+
+    typedef void (*fn_t)(bool*);
+    fn_t fn = reinterpret_cast<fn_t>(dlsym(handle, "init_thread_local_variable"));
+    ASSERT_TRUE(fn != nullptr) << dlerror();
+
+    fn(&is_dtor1_triggered);
+
+    lock.lock();
+    cv.wait(lock, [&] { return thread2_thread_local_dtor_initialized; });
+    lock.unlock();
+
+    dlclose(handle);
+
+    ASSERT_TRUE(!is_dtor1_triggered);
+
+    // Since we have thread_atexit dtors associated with handle - the library should
+    // still be availabe.
+    handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+    ASSERT_TRUE(handle != nullptr) << dlerror();
+    dlclose(handle);
+  };
+
+  auto f2 = [&]() {
+    std::unique_lock<std::mutex> lock(mtx);
+    cv.wait(lock, [&] { return thread1_dlopen_complete; });
+    void* handle = library_handle;
+    lock.unlock();
+
+    typedef void (*fn_t)(bool*);
+    fn_t fn = reinterpret_cast<fn_t>(dlsym(handle, "init_thread_local_variable2"));
+    ASSERT_TRUE(fn != nullptr) << dlerror();
+
+    fn(&is_dtor2_triggered);
+
+    lock.lock();
+    thread2_thread_local_dtor_initialized = true;
+    lock.unlock();
+    cv.notify_one();
+
+    lock.lock();
+    cv.wait(lock, [&] { return thread1_complete; });
+    lock.unlock();
+
+    ASSERT_TRUE(!is_dtor2_triggered);
+  };
+
+  void* handle = dlopen(library_name, RTLD_NOW);
+  ASSERT_TRUE(handle != nullptr) << dlerror();
+  dlclose(handle);
+
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+  ASSERT_TRUE(handle == nullptr);
+
+  std::thread t1(f1);
+  std::thread t2(f2);
+  t1.join();
+  ASSERT_TRUE(is_dtor1_triggered);
+  ASSERT_TRUE(!is_dtor2_triggered);
+
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+  ASSERT_TRUE(handle != nullptr) << dlerror();
+  dlclose(handle);
+
+  std::unique_lock<std::mutex> lock(mtx);
+  thread1_complete = true;
+  lock.unlock();
+  cv.notify_one();
+
+  t2.join();
+  ASSERT_TRUE(is_dtor2_triggered);
+
+#if defined(__BIONIC__)
+  // ld-android.so unloads unreferenced libraries on pthread_exit()
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
+  ASSERT_TRUE(handle == nullptr);
+#else
+  // GLIBC does not unload libraries with ref_count = 0 on pthread_exit
+  handle = dlopen(library_name, RTLD_NOW | RTLD_NOLOAD);
   ASSERT_TRUE(handle != nullptr) << dlerror();
 #endif
 }

tests/libs/Android.bp

@@ -700,7 +700,7 @@ cc_test_library {
 }
 
 // -----------------------------------------------------------------------------
-// Library with non-trivial thread_local variable to test dlclose()
+// Libraries with non-trivial thread_local variable to test dlclose()
 // -----------------------------------------------------------------------------
 cc_test_library {
     name: "libtest_thread_local_dtor",
@@ -708,6 +708,26 @@ cc_test_library {
     srcs: ["thread_local_dtor.cpp"],
 }
 
+cc_test_library {
+    name: "libtest_thread_local_dtor2",
+    defaults: ["bionic_testlib_defaults"],
+    srcs: ["thread_local_dtor2.cpp"],
+}
+
+// -----------------------------------------------------------------------------
+// Library dt_needs libtest_thread_local_dtor/2 (to check no-unload on load_group)
+// -----------------------------------------------------------------------------
+cc_test_library {
+    name: "libtest_indirect_thread_local_dtor",
+    defaults: ["bionic_testlib_defaults"],
+    srcs: ["empty.cpp"],
+    shared_libs: [
+        "libtest_thread_local_dtor",
+        "libtest_thread_local_dtor2",
+    ],
+}
+
+
 // -----------------------------------------------------------------------------
 // Tool to use to align the shared libraries in a zip file.
 // -----------------------------------------------------------------------------

tests/libs/thread_local_dtor2.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+namespace {
+
+class TestClass {
+ public:
+  TestClass(bool* flag) : flag_(flag) {}
+  ~TestClass() {
+    *flag_ = true;
+  }
+ private:
+  bool* flag_;
+};
+
+};  // namespace
+
+extern "C" void init_thread_local_variable2(bool* flag) {
+  thread_local TestClass test(flag);
+}