diff options
author | Andreas Gampe <agampe@google.com> | 2014-09-02 21:17:03 -0700 |
---|---|---|
committer | Andreas Gampe <agampe@google.com> | 2014-09-08 11:12:13 -0700 |
commit | 41df668c7be461f461b3d70951dee7634ded868f (patch) | |
tree | 18f63fba57fc7311a19f30027d2ea0063400d110 /libnativebridge | |
parent | 97b536f1fbfd1fa711833b7dc92aed902dea4bdf (diff) | |
download | core-41df668c7be461f461b3d70951dee7634ded868f.tar.gz core-41df668c7be461f461b3d70951dee7634ded868f.tar.bz2 core-41df668c7be461f461b3d70951dee7634ded868f.zip |
NativeBridge: Refactor for new initialization flow
Setup becomes Load, have explicit Initialize and Unload.
(cherry picked from commit 035bd7541ed909344348b6a4e17a7ef01a434653)
Change-Id: I5a20de1cb68dd1802937b369b14c50c9c1031c67
Diffstat (limited to 'libnativebridge')
-rw-r--r-- | libnativebridge/Android.mk | 2 | ||||
-rw-r--r-- | libnativebridge/native_bridge.cc | 182 | ||||
-rw-r--r-- | libnativebridge/tests/InvalidCharsNativeBridge_test.cpp | 2 | ||||
-rw-r--r-- | libnativebridge/tests/ReSetupNativeBridge_test.cpp | 6 | ||||
-rw-r--r-- | libnativebridge/tests/UnavailableNativeBridge_test.cpp | 5 | ||||
-rw-r--r-- | libnativebridge/tests/ValidNameNativeBridge_test.cpp | 12 |
6 files changed, 131 insertions, 78 deletions
diff --git a/libnativebridge/Android.mk b/libnativebridge/Android.mk index 9403fd2e1..3009bb959 100644 --- a/libnativebridge/Android.mk +++ b/libnativebridge/Android.mk @@ -36,3 +36,5 @@ LOCAL_LDFLAGS := -ldl 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 2205f453b..6602d3fa1 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. +}; + +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 a native bridge is available (loaded and ready). -static bool available = false; -// Whether we have already initialized (or tried to). -static bool initialized = false; // 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 @@ bool NativeBridgeNameAcceptable(const char* nb_library_filename) { } } -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; + return false; } - runtime_callbacks = runtime_cbs; - - if (nb_library_filename == nullptr) { - available = false; - initialized = true; + if (nb_library_filename == nullptr || *nb_library_filename == 0) { + state = NativeBridgeState::kClosed; + return 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; + if (!NativeBridgeNameAcceptable(nb_library_filename)) { + state = NativeBridgeState::kClosed; had_error = true; - } + } 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); + } + } - if (!initialized) { - // Didn't find a name error or empty string, assign it. - native_bridge_library_filename = nb_library_filename; + // 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; } } -static bool NativeBridgeInitialize() { - Mutex::Autolock auto_lock(native_bridge_lock); +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 (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; + 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 { had_error = true; - return false; + state = NativeBridgeState::kClosed; } - void* handle = dlopen(native_bridge_library_filename, RTLD_LAZY); - if (handle != nullptr) { - callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle, - kNativeBridgeInterfaceSymbol)); + return state == NativeBridgeState::kInitialized; +} - if (callbacks != nullptr) { - available = callbacks->initialize(runtime_callbacks); - } +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. - if (!available) { - // If we fail initialization, this counts as an error. + 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; - dlclose(handle); - } - } else { - // Being unable to open the library counts as an error. - had_error = true; - } + break; - initialized = true; + case NativeBridgeState::kClosed: + // Ignore. + break; + } - return available; + state = NativeBridgeState::kClosed; } bool NativeBridgeError() { @@ -165,11 +209,17 @@ bool NativeBridgeError() { } 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* NativeBridgeLoadLibrary(const char* libpath, int flag) { 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 f37e9c158..8f7973df0 100644 --- a/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp +++ b/libnativebridge/tests/InvalidCharsNativeBridge_test.cpp @@ -23,7 +23,7 @@ static const char* kTestName = "../librandom$@-bridge_not.existing.so"; 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 ef5bfceb8..944e5d7e9 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 27d12336c..ec96c32fe 100644 --- a/libnativebridge/tests/UnavailableNativeBridge_test.cpp +++ b/libnativebridge/tests/UnavailableNativeBridge_test.cpp @@ -20,9 +20,10 @@ namespace android { 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 3e019232d..690be4a30 100644 --- a/libnativebridge/tests/ValidNameNativeBridge_test.cpp +++ b/libnativebridge/tests/ValidNameNativeBridge_test.cpp @@ -21,13 +21,15 @@ namespace android { 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 |