NativeBridge: Refactor for new initialization flow
Setup becomes Load, have explicit Initialize and Unload.
(cherry picked from commit 035bd7541ed909344348b6a4e17a7ef01a434653)
Change-Id: I5a20de1cb68dd1802937b369b14c50c9c1031c67
diff --git a/include/nativebridge/native_bridge.h b/include/nativebridge/native_bridge.h
index c588bbc..16939f1 100644
--- a/include/nativebridge/native_bridge.h
+++ b/include/nativebridge/native_bridge.h
@@ -24,14 +24,24 @@
struct NativeBridgeRuntimeCallbacks;
-// Initialize the native bridge, if any. Should be called by Runtime::Init().
-// A null library filename signals that we do not want to load a native bridge.
-void SetupNativeBridge(const char* native_bridge_library_filename,
- const NativeBridgeRuntimeCallbacks* runtime_callbacks);
+// Open the native bridge, if any. Should be called by Runtime::Init(). A null library filename
+// signals that we do not want to load a native bridge.
+bool LoadNativeBridge(const char* native_bridge_library_filename,
+ const NativeBridgeRuntimeCallbacks* runtime_callbacks);
+
+// Initialize the native bridge, if any. Should be called by Runtime::DidForkFromZygote.
+bool InitializeNativeBridge();
+
+// Unload the native bridge, if any. Should be called by Runtime::DidForkFromZygote.
+void UnloadNativeBridge();
+
+// Check whether a native bridge is available (opened or initialized). Requires a prior call to
+// LoadNativeBridge.
+bool NativeBridgeAvailable();
// Check whether a native bridge is available (initialized). Requires a prior call to
-// SetupNativeBridge to make sense.
-bool NativeBridgeAvailable();
+// LoadNativeBridge & InitializeNativeBridge.
+bool NativeBridgeInitialized();
// Load a shared library that is supported by the native bridge.
void* NativeBridgeLoadLibrary(const char* libpath, int flag);
diff --git a/libnativebridge/Android.mk b/libnativebridge/Android.mk
index 9403fd2..3009bb9 100644
--- a/libnativebridge/Android.mk
+++ b/libnativebridge/Android.mk
@@ -36,3 +36,5 @@
LOCAL_MULTILIB := both
include $(BUILD_HOST_SHARED_LIBRARY)
+
+include $(LOCAL_PATH)/tests/Android.mk
\ No newline at end of file
diff --git a/libnativebridge/native_bridge.cc b/libnativebridge/native_bridge.cc
index 2205f45..6602d3f 100644
--- a/libnativebridge/native_bridge.cc
+++ b/libnativebridge/native_bridge.cc
@@ -19,27 +19,53 @@
#include <cutils/log.h>
#include <dlfcn.h>
#include <stdio.h>
-#include "utils/Mutex.h"
namespace android {
-static Mutex native_bridge_lock("native bridge lock");
-
// The symbol name exposed by native-bridge with the type of NativeBridgeCallbacks.
static constexpr const char* kNativeBridgeInterfaceSymbol = "NativeBridgeItf";
-// The filename of the library we are supposed to load.
-static const char* native_bridge_library_filename = nullptr;
+enum class NativeBridgeState {
+ kNotSetup, // Initial state.
+ kOpened, // After successful dlopen.
+ kInitialized, // After successful initialization.
+ kClosed // Closed or errors.
+};
-// Whether a native bridge is available (loaded and ready).
-static bool available = false;
-// Whether we have already initialized (or tried to).
-static bool initialized = false;
+static const char* kNotSetupString = "kNotSetup";
+static const char* kOpenedString = "kOpened";
+static const char* kInitializedString = "kInitialized";
+static const char* kClosedString = "kClosed";
+
+static const char* GetNativeBridgeStateString(NativeBridgeState state) {
+ switch (state) {
+ case NativeBridgeState::kNotSetup:
+ return kNotSetupString;
+
+ case NativeBridgeState::kOpened:
+ return kOpenedString;
+
+ case NativeBridgeState::kInitialized:
+ return kInitializedString;
+
+ case NativeBridgeState::kClosed:
+ return kClosedString;
+ }
+}
+
+// Current state of the native bridge.
+static NativeBridgeState state = NativeBridgeState::kNotSetup;
+
// Whether we had an error at some point.
static bool had_error = false;
+// Handle of the loaded library.
+static void* native_bridge_handle = nullptr;
+// Pointer to the callbacks. Available as soon as LoadNativeBridge succeeds, but only initialized
+// later.
static NativeBridgeCallbacks* callbacks = nullptr;
+// Callbacks provided by the environment to the bridge. Passed to LoadNativeBridge.
static const NativeBridgeRuntimeCallbacks* runtime_callbacks = nullptr;
// Characters allowed in a native bridge filename. The first character must
@@ -83,81 +109,99 @@
}
}
-void SetupNativeBridge(const char* nb_library_filename,
- const NativeBridgeRuntimeCallbacks* runtime_cbs) {
- Mutex::Autolock auto_lock(native_bridge_lock);
+bool LoadNativeBridge(const char* nb_library_filename,
+ const NativeBridgeRuntimeCallbacks* runtime_cbs) {
+ // We expect only one place that calls LoadNativeBridge: Runtime::Init. At that point we are not
+ // multi-threaded, so we do not need locking here.
- if (initialized || native_bridge_library_filename != nullptr) {
+ if (state != NativeBridgeState::kNotSetup) {
// Setup has been called before. Ignore this call.
- ALOGW("Called SetupNativeBridge for an already set up native bridge.");
+ ALOGW("Called LoadNativeBridge for an already set up native bridge. State is %s.",
+ GetNativeBridgeStateString(state));
// Note: counts as an error, even though the bridge may be functional.
had_error = true;
- return;
- }
-
- runtime_callbacks = runtime_cbs;
-
- if (nb_library_filename == nullptr) {
- available = false;
- initialized = true;
- } else {
- // Check whether it's an empty string.
- if (*nb_library_filename == 0) {
- available = false;
- initialized = true;
- } else if (!NativeBridgeNameAcceptable(nb_library_filename)) {
- available = false;
- initialized = true;
- had_error = true;
- }
-
- if (!initialized) {
- // Didn't find a name error or empty string, assign it.
- native_bridge_library_filename = nb_library_filename;
- }
- }
-}
-
-static bool NativeBridgeInitialize() {
- Mutex::Autolock auto_lock(native_bridge_lock);
-
- if (initialized) {
- // Somebody did it before.
- return available;
- }
-
- available = false;
-
- if (native_bridge_library_filename == nullptr) {
- // Called initialize without setup. dlopen has special semantics for nullptr input.
- // So just call it a day here. This counts as an error.
- initialized = true;
- had_error = true;
return false;
}
- void* handle = dlopen(native_bridge_library_filename, RTLD_LAZY);
- if (handle != nullptr) {
- callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle,
- kNativeBridgeInterfaceSymbol));
-
- if (callbacks != nullptr) {
- available = callbacks->initialize(runtime_callbacks);
- }
-
- if (!available) {
- // If we fail initialization, this counts as an error.
+ if (nb_library_filename == nullptr || *nb_library_filename == 0) {
+ state = NativeBridgeState::kClosed;
+ return true;
+ } else {
+ if (!NativeBridgeNameAcceptable(nb_library_filename)) {
+ state = NativeBridgeState::kClosed;
had_error = true;
- dlclose(handle);
+ } else {
+ // Try to open the library.
+ void* handle = dlopen(nb_library_filename, RTLD_LAZY);
+ if (handle != nullptr) {
+ callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle,
+ kNativeBridgeInterfaceSymbol));
+ if (callbacks != nullptr) {
+ // Store the handle for later.
+ native_bridge_handle = handle;
+ } else {
+ dlclose(handle);
+ }
+ }
+
+ // Two failure conditions: could not find library (dlopen failed), or could not find native
+ // bridge interface (dlsym failed). Both are an error and close the native bridge.
+ if (callbacks == nullptr) {
+ had_error = true;
+ state = NativeBridgeState::kClosed;
+ } else {
+ runtime_callbacks = runtime_cbs;
+ state = NativeBridgeState::kOpened;
+ }
+ }
+ return state == NativeBridgeState::kOpened;
+ }
+}
+
+bool InitializeNativeBridge() {
+ // We expect only one place that calls InitializeNativeBridge: Runtime::DidForkFromZygote. At that
+ // point we are not multi-threaded, so we do not need locking here.
+
+ if (state == NativeBridgeState::kOpened) {
+ // Try to initialize.
+ if (callbacks->initialize(runtime_callbacks)) {
+ state = NativeBridgeState::kInitialized;
+ } else {
+ // Unload the library.
+ dlclose(native_bridge_handle);
+ had_error = true;
+ state = NativeBridgeState::kClosed;
}
} else {
- // Being unable to open the library counts as an error.
had_error = true;
+ state = NativeBridgeState::kClosed;
}
- initialized = true;
+ return state == NativeBridgeState::kInitialized;
+}
- return available;
+void UnloadNativeBridge() {
+ // We expect only one place that calls UnloadNativeBridge: Runtime::DidForkFromZygote. At that
+ // point we are not multi-threaded, so we do not need locking here.
+
+ switch(state) {
+ case NativeBridgeState::kOpened:
+ case NativeBridgeState::kInitialized:
+ // Unload.
+ dlclose(native_bridge_handle);
+ break;
+
+ case NativeBridgeState::kNotSetup:
+ // Not even set up. Error.
+ had_error = true;
+ break;
+
+ case NativeBridgeState::kClosed:
+ // Ignore.
+ break;
+ }
+
+ state = NativeBridgeState::kClosed;
}
bool NativeBridgeError() {
@@ -165,11 +209,17 @@
}
bool NativeBridgeAvailable() {
- return NativeBridgeInitialize();
+ return state == NativeBridgeState::kOpened || state == NativeBridgeState::kInitialized;
+}
+
+bool NativeBridgeInitialized() {
+ // Calls of this are supposed to happen in a state where the native bridge is stable, i.e., after
+ // Runtime::DidForkFromZygote. In that case we do not need a lock.
+ return state == NativeBridgeState::kInitialized;
}
void* NativeBridgeLoadLibrary(const char* libpath, int flag) {
- if (NativeBridgeInitialize()) {
+ if (NativeBridgeInitialized()) {
return callbacks->loadLibrary(libpath, flag);
}
return nullptr;
@@ -177,14 +227,14 @@
void* NativeBridgeGetTrampoline(void* handle, const char* name, const char* shorty,
uint32_t len) {
- if (NativeBridgeInitialize()) {
+ if (NativeBridgeInitialized()) {
return callbacks->getTrampoline(handle, name, shorty, len);
}
return nullptr;
}
bool NativeBridgeIsSupported(const char* libpath) {
- if (NativeBridgeInitialize()) {
+ if (NativeBridgeInitialized()) {
return callbacks->isSupported(libpath);
}
return false;
diff --git a/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp b/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp
index f37e9c1..8f7973d 100644
--- a/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp
+++ b/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp
@@ -23,7 +23,7 @@
TEST_F(NativeBridgeTest, InvalidChars) {
// Do one test actually calling setup.
EXPECT_EQ(false, NativeBridgeError());
- SetupNativeBridge(kTestName, nullptr);
+ LoadNativeBridge(kTestName, nullptr);
// This should lead to an error for invalid characters.
EXPECT_EQ(true, NativeBridgeError());
diff --git a/libnativebridge/tests/ReSetupNativeBridge_test.cpp b/libnativebridge/tests/ReSetupNativeBridge_test.cpp
index ef5bfce..944e5d7 100644
--- a/libnativebridge/tests/ReSetupNativeBridge_test.cpp
+++ b/libnativebridge/tests/ReSetupNativeBridge_test.cpp
@@ -18,13 +18,11 @@
namespace android {
-static const char* kTestName = "librandom-bridge_not.existing.so";
-
TEST_F(NativeBridgeTest, ReSetup) {
EXPECT_EQ(false, NativeBridgeError());
- SetupNativeBridge(kTestName, nullptr);
+ LoadNativeBridge("", nullptr);
EXPECT_EQ(false, NativeBridgeError());
- SetupNativeBridge(kTestName, nullptr);
+ LoadNativeBridge("", nullptr);
// This should lead to an error for trying to re-setup a native bridge.
EXPECT_EQ(true, NativeBridgeError());
}
diff --git a/libnativebridge/tests/UnavailableNativeBridge_test.cpp b/libnativebridge/tests/UnavailableNativeBridge_test.cpp
index 27d1233..ec96c32 100644
--- a/libnativebridge/tests/UnavailableNativeBridge_test.cpp
+++ b/libnativebridge/tests/UnavailableNativeBridge_test.cpp
@@ -20,9 +20,10 @@
TEST_F(NativeBridgeTest, NoNativeBridge) {
EXPECT_EQ(false, NativeBridgeAvailable());
- // This should lead to an error for trying to initialize a not-setup
- // native bridge.
+ // Try to initialize. This should fail as we are not set up.
+ EXPECT_EQ(false, InitializeNativeBridge());
EXPECT_EQ(true, NativeBridgeError());
+ EXPECT_EQ(false, NativeBridgeAvailable());
}
} // namespace android
diff --git a/libnativebridge/tests/ValidNameNativeBridge_test.cpp b/libnativebridge/tests/ValidNameNativeBridge_test.cpp
index 3e01923..690be4a 100644
--- a/libnativebridge/tests/ValidNameNativeBridge_test.cpp
+++ b/libnativebridge/tests/ValidNameNativeBridge_test.cpp
@@ -21,13 +21,15 @@
static const char* kTestName = "librandom-bridge_not.existing.so";
TEST_F(NativeBridgeTest, ValidName) {
+ // Check that the name is acceptable.
+ EXPECT_EQ(true, NativeBridgeNameAcceptable(kTestName));
+
+ // Now check what happens on LoadNativeBridge.
EXPECT_EQ(false, NativeBridgeError());
- SetupNativeBridge(kTestName, nullptr);
- EXPECT_EQ(false, NativeBridgeError());
- EXPECT_EQ(false, NativeBridgeAvailable());
- // This should lead to an error for trying to initialize a not-existing
- // native bridge.
+ LoadNativeBridge(kTestName, nullptr);
+ // This will lead to an error as the library doesn't exist.
EXPECT_EQ(true, NativeBridgeError());
+ EXPECT_EQ(false, NativeBridgeAvailable());
}
} // namespace android