[native bridge] Make sure we always unload the native bridge

libnativebridge may allocate some resources during loading and
intialization and de-allocate them when unloading. This makes sure that
we don't leak anything.

Bug: 18097480

(cherry picked from commit 07d83c7a25022064ac0a8dac4fe2a7a38681fa4b)

Change-Id: I3a5155a6760399ccfaf9130c72679615a3d4f2e7

4 files changed, 29 insertions, 21 deletions

diff --git a/runtime/native_bridge_art_interface.cc b/runtime/native_bridge_art_interface.cc
index cc44615..28eebdd 100644
--- a/runtime/native_bridge_art_interface.cc
+++ b/runtime/native_bridge_art_interface.cc
@@ -107,10 +107,11 @@ static android::NativeBridgeRuntimeCallbacks native_bridge_art_callbacks_ {
   GetMethodShorty, GetNativeMethodCount, GetNativeMethods
 };
 
-void LoadNativeBridge(std::string& native_bridge_library_filename) {
-  android::LoadNativeBridge(native_bridge_library_filename.c_str(), &native_bridge_art_callbacks_);
+bool LoadNativeBridge(std::string& native_bridge_library_filename) {
   VLOG(startup) << "Runtime::Setup native bridge library: "
       << (native_bridge_library_filename.empty() ? "(empty)" : native_bridge_library_filename);
+  return android::LoadNativeBridge(native_bridge_library_filename.c_str(),
+                                   &native_bridge_art_callbacks_);
 }
 
 void PreInitializeNativeBridge(std::string dir) {
diff --git a/runtime/native_bridge_art_interface.h b/runtime/native_bridge_art_interface.h
index 42f0ed2..c287f3b 100644
--- a/runtime/native_bridge_art_interface.h
+++ b/runtime/native_bridge_art_interface.h
@@ -26,7 +26,7 @@ namespace art {
 // Mirror libnativebridge interface. Done to have the ART callbacks out of line, and not require
 // the system/core header file in other files.
 
-void LoadNativeBridge(std::string& native_bridge_library_filename);
+bool LoadNativeBridge(std::string& native_bridge_library_filename);
 
 // This is mostly for testing purposes, as in a full system this is called by Zygote code.
 void PreInitializeNativeBridge(std::string dir);
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 9848382..4b0918d 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -147,10 +147,14 @@ Runtime::Runtime()
       target_sdk_version_(0),
       implicit_null_checks_(false),
       implicit_so_checks_(false),
-      implicit_suspend_checks_(false) {
+      implicit_suspend_checks_(false),
+      is_native_bridge_loaded_(false) {
 }
 
 Runtime::~Runtime() {
+  if (is_native_bridge_loaded_) {
+    UnloadNativeBridge();
+  }
   if (dump_gc_performance_on_shutdown_) {
     // This can't be called from the Heap destructor below because it
     // could call RosAlloc::InspectAll() which needs the thread_list
@@ -434,12 +438,11 @@ bool Runtime::Start() {
       return false;
     }
   } else {
-    bool have_native_bridge = !native_bridge_library_filename_.empty();
-    if (have_native_bridge) {
+    if (is_native_bridge_loaded_) {
       PreInitializeNativeBridge(".");
     }
-    DidForkFromZygote(self->GetJniEnv(), have_native_bridge ? NativeBridgeAction::kInitialize :
-        NativeBridgeAction::kUnload, GetInstructionSetString(kRuntimeISA));
+    DidForkFromZygote(self->GetJniEnv(), NativeBridgeAction::kInitialize,
+                      GetInstructionSetString(kRuntimeISA));
   }
 
   StartDaemonThreads();
@@ -518,14 +521,17 @@ bool Runtime::InitZygote() {
 void Runtime::DidForkFromZygote(JNIEnv* env, NativeBridgeAction action, const char* isa) {
   is_zygote_ = false;
 
-  switch (action) {
-    case NativeBridgeAction::kUnload:
-      UnloadNativeBridge();
-      break;
+  if (is_native_bridge_loaded_) {
+    switch (action) {
+      case NativeBridgeAction::kUnload:
+        UnloadNativeBridge();
+        is_native_bridge_loaded_ = false;
+        break;
 
-    case NativeBridgeAction::kInitialize:
-      InitializeNativeBridge(env, isa);
-      break;
+      case NativeBridgeAction::kInitialize:
+        InitializeNativeBridge(env, isa);
+        break;
+    }
   }
 
   // Create the thread pool.
@@ -882,8 +888,7 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
   // Runtime::Start():
   //   DidForkFromZygote(kInitialize) -> try to initialize any native bridge given.
   //   No-op wrt native bridge.
-  native_bridge_library_filename_ = options->native_bridge_library_filename_;
-  LoadNativeBridge(native_bridge_library_filename_);
+  is_native_bridge_loaded_ = LoadNativeBridge(options->native_bridge_library_filename_);
 
   VLOG(startup) << "Runtime::Init exiting";
   return true;
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 8cfa8aa..5fe4b84 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -638,14 +638,16 @@ class Runtime {
   bool implicit_so_checks_;         // StackOverflow checks are implicit.
   bool implicit_suspend_checks_;    // Thread suspension checks are implicit.
 
-  // The filename to the native bridge library. If this is not empty the native bridge will be
-  // initialized and loaded from the given file (initialized and available). An empty value means
-  // that there's no native bridge (initialized but not available).
+  // Whether or not a native bridge has been loaded.
   //
   // The native bridge allows running native code compiled for a foreign ISA. The way it works is,
   // if standard dlopen fails to load native library associated with native activity, it calls to
   // the native bridge to load it and then gets the trampoline for the entry to native activity.
-  std::string native_bridge_library_filename_;
+  //
+  // The option 'native_bridge_library_filename' specifies the name of the native bridge.
+  // When non-empty the native bridge will be loaded from the given file. An empty value means
+  // that there's no native bridge.
+  bool is_native_bridge_loaded_;
 
   DISALLOW_COPY_AND_ASSIGN(Runtime);
 };