Merge "Revert "Fix bug with double unload on unsuccessful dlopen""

4 files changed, 11 insertions, 94 deletions

diff --git a/linker/linker.cpp b/linker/linker.cpp
index f1ef55788..85376e0f7 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -321,7 +321,9 @@ static void soinfo_free(soinfo* si) {
   TRACE("name %s: freeing soinfo @ %p", si->get_realpath(), si);
 
   if (!solist_remove_soinfo(si)) {
-    async_safe_fatal("soinfo=%p is not in soinfo_list (double unload?)", si);
+    // TODO (dimitry): revisit this - for now preserving the logic
+    // but it does not look right, abort if soinfo is not in the list instead?
+    return;
   }
 
   // clear links to/from si
@@ -1518,6 +1520,11 @@ bool find_libraries(android_namespace_t* ns,
     }
   });
 
+  auto failure_guard = android::base::make_scope_guard([&]() {
+    // Housekeeping
+    soinfo_unload(soinfos, soinfos_count);
+  });
+
   ZipArchiveCache zip_archive_cache;
 
   // Step 1: expand the list of load_tasks to include
@@ -1682,6 +1689,8 @@ bool find_libraries(android_namespace_t* ns,
         si->set_linked();
       }
     });
+
+    failure_guard.Disable();
   }
 
   return linked;
@@ -1691,7 +1700,7 @@ static soinfo* find_library(android_namespace_t* ns,
                             const char* name, int rtld_flags,
                             const android_dlextinfo* extinfo,
                             soinfo* needed_by) {
-  soinfo* si = nullptr;
+  soinfo* si;
 
   // readers_map is shared across recursive calls to find_libraries.
   // However, the map is not shared across different threads.
@@ -1710,9 +1719,6 @@ static soinfo* find_library(android_namespace_t* ns,
                              false /* add_as_children */,
                              true /* search_linked_namespaces */,
                              readers_map)) {
-    if (si != nullptr) {
-      soinfo_unload(si);
-    }
     return nullptr;
   }
 
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 31aadcad1..3f6da59ca 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -1051,46 +1051,6 @@ TEST(dlext, ns_unload_between_namespaces) {
             "\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror());
 }
 
-TEST(dlext, ns_unload_between_namespaces_missing_symbol) {
-  ASSERT_TRUE(android_init_anonymous_namespace(g_core_shared_libs.c_str(), nullptr));
-
-  const std::string public_ns_search_path =  get_testlib_root() + "/public_namespace_libs";
-  const std::string private_ns_search_path = get_testlib_root() + "/private_namespace_libs";
-
-  android_namespace_t* ns_public =
-          android_create_namespace("public",
-                                   nullptr,
-                                   public_ns_search_path.c_str(),
-                                   ANDROID_NAMESPACE_TYPE_ISOLATED,
-                                   nullptr,
-                                   nullptr);
-
-  ASSERT_TRUE(android_link_namespaces(ns_public, nullptr, g_core_shared_libs.c_str())) << dlerror();
-
-  android_namespace_t* ns_private =
-          android_create_namespace("private",
-                                   nullptr,
-                                   private_ns_search_path.c_str(),
-                                   ANDROID_NAMESPACE_TYPE_ISOLATED,
-                                   nullptr,
-                                   nullptr);
-
-  ASSERT_TRUE(android_link_namespaces(ns_private, ns_public, "libtest_missing_symbol.so")) << dlerror();
-  ASSERT_TRUE(android_link_namespaces(ns_private, nullptr, g_core_shared_libs.c_str())) << dlerror();
-
-  android_dlextinfo extinfo;
-  extinfo.flags = ANDROID_DLEXT_USE_NAMESPACE;
-  extinfo.library_namespace = ns_private;
-
-  void* handle = android_dlopen_ext((public_ns_search_path + "/libtest_missing_symbol.so").c_str(),
-                                    RTLD_NOW,
-                                    &extinfo);
-  ASSERT_TRUE(handle == nullptr);
-  ASSERT_EQ(std::string("dlopen failed: cannot locate symbol \"dlopen_testlib_missing_symbol\" referenced by \"") +
-            public_ns_search_path + "/libtest_missing_symbol.so\"...",
-            dlerror());
-}
-
 TEST(dlext, ns_greylist_enabled) {
   ASSERT_TRUE(android_init_anonymous_namespace(g_core_shared_libs.c_str(), nullptr));
 
diff --git a/tests/libs/Android.bp b/tests/libs/Android.bp
index 392823d58..8d0271aae 100644
--- a/tests/libs/Android.bp
+++ b/tests/libs/Android.bp
@@ -113,18 +113,6 @@ cc_test_library {
 }
 
 // -----------------------------------------------------------------------------
-// Library used by dlfcn unload tests
-// -----------------------------------------------------------------------------
-cc_test_library {
-    name: "libtest_missing_symbol",
-    defaults: ["bionic_testlib_defaults"],
-    srcs: ["dlopen_testlib_missing_symbol.cpp"],
-    allow_undefined_symbols: true,
-    relative_install_path: "bionic-loader-test-libs/public_namespace_libs",
-}
-
-// -----------------------------------------------------------------------------
-// -----------------------------------------------------------------------------
 // Library used by dlfcn nodelete tests
 // -----------------------------------------------------------------------------
 cc_test_library {
diff --git a/tests/libs/dlopen_testlib_missing_symbol.cpp b/tests/libs/dlopen_testlib_missing_symbol.cpp
deleted file mode 100644
index 0f73c604c..000000000
--- a/tests/libs/dlopen_testlib_missing_symbol.cpp
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Copyright (C) 2017 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.
- */
-
-#include <stdint.h>
-#include <stdlib.h>
-
-extern "C" void dlopen_testlib_missing_symbol();
-
-extern "C" bool dlopen_testlib_simple_func() {
-  dlopen_testlib_missing_symbol();
-  return true;
-}