Merge "Handle code_cache dir creation in libnativebridge" into lmp-mr1-dev
automerge: 9c094da
* commit '9c094da83112bbcad37eaffecfee96ff169d84a8':
Handle code_cache dir creation in libnativebridge
diff --git a/include/nativebridge/native_bridge.h b/include/nativebridge/native_bridge.h
index ac254e9..523dc49 100644
--- a/include/nativebridge/native_bridge.h
+++ b/include/nativebridge/native_bridge.h
@@ -37,7 +37,7 @@
// Do the early initialization part of the native bridge, if necessary. This should be done under
// high privileges.
-void PreInitializeNativeBridge(const char* app_data_dir, const char* instruction_set);
+bool PreInitializeNativeBridge(const char* app_data_dir, const char* instruction_set);
// Initialize the native bridge, if any. Should be called by Runtime::DidForkFromZygote. The JNIEnv*
// will be used to modify the app environment for the bridge.
diff --git a/libnativebridge/Android.mk b/libnativebridge/Android.mk
index 6c2e43e..83169eb 100644
--- a/libnativebridge/Android.mk
+++ b/libnativebridge/Android.mk
@@ -13,7 +13,7 @@
LOCAL_SHARED_LIBRARIES := liblog
LOCAL_CLANG := true
LOCAL_CPP_EXTENSION := .cc
-LOCAL_CFLAGS := -Werror -Wall
+LOCAL_CFLAGS += -Werror -Wall
LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
LOCAL_LDFLAGS := -ldl
LOCAL_MULTILIB := both
@@ -30,11 +30,11 @@
LOCAL_SHARED_LIBRARIES := liblog
LOCAL_CLANG := true
LOCAL_CPP_EXTENSION := .cc
-LOCAL_CFLAGS := -Werror -Wall
+LOCAL_CFLAGS += -Werror -Wall
LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
LOCAL_LDFLAGS := -ldl
LOCAL_MULTILIB := both
include $(BUILD_HOST_SHARED_LIBRARY)
-include $(LOCAL_PATH)/tests/Android.mk
\ No newline at end of file
+include $(LOCAL_PATH)/tests/Android.mk
diff --git a/libnativebridge/native_bridge.cc b/libnativebridge/native_bridge.cc
index 3facedd..eab2de0 100644
--- a/libnativebridge/native_bridge.cc
+++ b/libnativebridge/native_bridge.cc
@@ -43,14 +43,16 @@
enum class NativeBridgeState {
kNotSetup, // Initial state.
kOpened, // After successful dlopen.
+ kPreInitialized, // After successful pre-initialization.
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 constexpr const char* kNotSetupString = "kNotSetup";
+static constexpr const char* kOpenedString = "kOpened";
+static constexpr const char* kPreInitializedString = "kPreInitialized";
+static constexpr const char* kInitializedString = "kInitialized";
+static constexpr const char* kClosedString = "kClosed";
static const char* GetNativeBridgeStateString(NativeBridgeState state) {
switch (state) {
@@ -60,6 +62,9 @@
case NativeBridgeState::kOpened:
return kOpenedString;
+ case NativeBridgeState::kPreInitialized:
+ return kPreInitializedString;
+
case NativeBridgeState::kInitialized:
return kInitializedString;
@@ -82,8 +87,14 @@
// Callbacks provided by the environment to the bridge. Passed to LoadNativeBridge.
static const NativeBridgeRuntimeCallbacks* runtime_callbacks = nullptr;
-// The app's data directory.
-static char* app_data_dir = nullptr;
+// The app's code cache directory.
+static char* app_code_cache_dir = nullptr;
+
+// Code cache directory (relative to the application private directory)
+// Ideally we'd like to call into framework to retrieve this name. However that's considered an
+// implementation detail and will require either hacks or consistent refactorings. We compromise
+// and hard code the directory name again here.
+static constexpr const char* kCodeCacheDir = "code_cache";
static constexpr uint32_t kNativeBridgeCallbackVersion = 1;
@@ -132,6 +143,13 @@
return cb != nullptr && cb->version == kNativeBridgeCallbackVersion;
}
+static void CloseNativeBridge(bool with_error) {
+ state = NativeBridgeState::kClosed;
+ had_error |= with_error;
+ delete[] app_code_cache_dir;
+ app_code_cache_dir = nullptr;
+}
+
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
@@ -149,12 +167,11 @@
}
if (nb_library_filename == nullptr || *nb_library_filename == 0) {
- state = NativeBridgeState::kClosed;
- return true;
+ CloseNativeBridge(false);
+ return false;
} else {
if (!NativeBridgeNameAcceptable(nb_library_filename)) {
- state = NativeBridgeState::kClosed;
- had_error = true;
+ CloseNativeBridge(true);
} else {
// Try to open the library.
void* handle = dlopen(nb_library_filename, RTLD_LAZY);
@@ -178,8 +195,7 @@
// 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;
+ CloseNativeBridge(true);
} else {
runtime_callbacks = runtime_cbs;
state = NativeBridgeState::kOpened;
@@ -216,19 +232,32 @@
template<typename T> void UNUSED(const T&) {}
#endif
-void PreInitializeNativeBridge(const char* app_data_dir_in, const char* instruction_set) {
- if (app_data_dir_in == nullptr) {
- return;
+bool PreInitializeNativeBridge(const char* app_data_dir_in, const char* instruction_set) {
+ if (state != NativeBridgeState::kOpened) {
+ ALOGE("Invalid state: native bridge is expected to be opened.");
+ CloseNativeBridge(true);
+ return false;
}
- const size_t len = strlen(app_data_dir_in);
- // Make a copy for us.
- app_data_dir = new char[len];
- strncpy(app_data_dir, app_data_dir_in, len);
+ if (app_data_dir_in == nullptr) {
+ ALOGE("Application private directory cannot be null.");
+ CloseNativeBridge(true);
+ return false;
+ }
+
+ // Create the path to the application code cache directory.
+ // The memory will be release after Initialization or when the native bridge is closed.
+ const size_t len = strlen(app_data_dir_in) + strlen(kCodeCacheDir) + 2; // '\0' + '/'
+ app_code_cache_dir = new char[len];
+ snprintf(app_code_cache_dir, len, "%s/%s", app_data_dir_in, kCodeCacheDir);
+
+ // Bind-mount /system/lib{,64}/<isa>/cpuinfo to /proc/cpuinfo.
+ // Failure is not fatal and will keep the native bridge in kPreInitialized.
+ state = NativeBridgeState::kPreInitialized;
#ifndef __APPLE__
if (instruction_set == nullptr) {
- return;
+ return true;
}
size_t isa_len = strlen(instruction_set);
if (isa_len > 10) {
@@ -237,11 +266,11 @@
// be another instruction set in the future.
ALOGW("Instruction set %s is malformed, must be less than or equal to 10 characters.",
instruction_set);
- return;
+ return true;
}
- // Bind-mount /system/lib{,64}/<isa>/cpuinfo to /proc/cpuinfo. If the file does not exist, the
- // mount command will fail, so we safe the extra file existence check...
+ // If the file does not exist, the mount command will fail,
+ // so we save the extra file existence check.
char cpuinfo_path[1024];
#ifdef HAVE_ANDROID_OS
@@ -263,10 +292,12 @@
nullptr)) == -1) { // "Data."
ALOGW("Failed to bind-mount %s as /proc/cpuinfo: %s", cpuinfo_path, strerror(errno));
}
-#else
+#else // __APPLE__
UNUSED(instruction_set);
ALOGW("Mac OS does not support bind-mounting. Host simulation of native bridge impossible.");
#endif
+
+ return true;
}
static void SetCpuAbi(JNIEnv* env, jclass build_class, const char* field, const char* value) {
@@ -353,20 +384,40 @@
// 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, app_data_dir, instruction_set)) {
- SetupEnvironment(callbacks, env, instruction_set);
- state = NativeBridgeState::kInitialized;
- } else {
- // Unload the library.
- dlclose(native_bridge_handle);
- had_error = true;
- state = NativeBridgeState::kClosed;
+ if (state == NativeBridgeState::kPreInitialized) {
+ // Check for code cache: if it doesn't exist try to create it.
+ struct stat st;
+ if (stat(app_code_cache_dir, &st) == -1) {
+ if (errno == ENOENT) {
+ if (mkdir(app_code_cache_dir, S_IRWXU | S_IRWXG | S_IXOTH) == -1) {
+ ALOGE("Cannot create code cache directory %s: %s.", app_code_cache_dir, strerror(errno));
+ CloseNativeBridge(true);
+ }
+ } else {
+ ALOGE("Cannot stat code cache directory %s: %s.", app_code_cache_dir, strerror(errno));
+ CloseNativeBridge(true);
+ }
+ } else if (!S_ISDIR(st.st_mode)) {
+ ALOGE("Code cache is not a directory %s.", app_code_cache_dir);
+ CloseNativeBridge(true);
+ }
+
+ // If we're still PreInitialized (dind't fail the code cache checks) try to initialize.
+ if (state == NativeBridgeState::kPreInitialized) {
+ if (callbacks->initialize(runtime_callbacks, app_code_cache_dir, instruction_set)) {
+ SetupEnvironment(callbacks, env, instruction_set);
+ state = NativeBridgeState::kInitialized;
+ // We no longer need the code cache path, release the memory.
+ delete[] app_code_cache_dir;
+ app_code_cache_dir = nullptr;
+ } else {
+ // Unload the library.
+ dlclose(native_bridge_handle);
+ CloseNativeBridge(true);
+ }
}
} else {
- had_error = true;
- state = NativeBridgeState::kClosed;
+ CloseNativeBridge(true);
}
return state == NativeBridgeState::kInitialized;
@@ -378,22 +429,22 @@
switch(state) {
case NativeBridgeState::kOpened:
+ case NativeBridgeState::kPreInitialized:
case NativeBridgeState::kInitialized:
// Unload.
dlclose(native_bridge_handle);
+ CloseNativeBridge(false);
break;
case NativeBridgeState::kNotSetup:
// Not even set up. Error.
- had_error = true;
+ CloseNativeBridge(true);
break;
case NativeBridgeState::kClosed:
// Ignore.
break;
}
-
- state = NativeBridgeState::kClosed;
}
bool NativeBridgeError() {
@@ -401,7 +452,9 @@
}
bool NativeBridgeAvailable() {
- return state == NativeBridgeState::kOpened || state == NativeBridgeState::kInitialized;
+ return state == NativeBridgeState::kOpened
+ || state == NativeBridgeState::kPreInitialized
+ || state == NativeBridgeState::kInitialized;
}
bool NativeBridgeInitialized() {
diff --git a/libnativebridge/tests/Android.mk b/libnativebridge/tests/Android.mk
index 9c7e1b8..3373473 100644
--- a/libnativebridge/tests/Android.mk
+++ b/libnativebridge/tests/Android.mk
@@ -1,19 +1,27 @@
# Build the unit tests.
LOCAL_PATH := $(call my-dir)
+
+include $(LOCAL_PATH)/Android.nativebridge-dummy.mk
+
include $(CLEAR_VARS)
# Build the unit tests.
test_src_files := \
+ CompleteFlow_test.cpp \
InvalidCharsNativeBridge_test.cpp \
NeedsNativeBridge_test.cpp \
PreInitializeNativeBridge_test.cpp \
+ PreInitializeNativeBridgeFail1_test.cpp \
+ PreInitializeNativeBridgeFail2_test.cpp \
ReSetupNativeBridge_test.cpp \
UnavailableNativeBridge_test.cpp \
ValidNameNativeBridge_test.cpp
+
shared_libraries := \
liblog \
- libnativebridge
+ libnativebridge \
+ libnativebridge-dummy
$(foreach file,$(test_src_files), \
$(eval include $(CLEAR_VARS)) \
@@ -33,4 +41,4 @@
$(eval LOCAL_SRC_FILES := $(file)) \
$(eval LOCAL_MODULE := $(notdir $(file:%.cpp=%))) \
$(eval include $(BUILD_HOST_NATIVE_TEST)) \
-)
\ No newline at end of file
+)
diff --git a/libnativebridge/tests/Android.nativebridge-dummy.mk b/libnativebridge/tests/Android.nativebridge-dummy.mk
new file mode 100644
index 0000000..1caf50a
--- /dev/null
+++ b/libnativebridge/tests/Android.nativebridge-dummy.mk
@@ -0,0 +1,34 @@
+LOCAL_PATH:= $(call my-dir)
+
+NATIVE_BRIDGE_COMMON_SRC_FILES := \
+ DummyNativeBridge.cpp
+
+# Shared library for target
+# ========================================================
+include $(CLEAR_VARS)
+
+LOCAL_MODULE:= libnativebridge-dummy
+
+LOCAL_SRC_FILES:= $(NATIVE_BRIDGE_COMMON_SRC_FILES)
+LOCAL_CLANG := true
+LOCAL_CFLAGS += -Werror -Wall
+LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
+LOCAL_LDFLAGS := -ldl
+LOCAL_MULTILIB := both
+
+include $(BUILD_SHARED_LIBRARY)
+
+# Shared library for host
+# ========================================================
+include $(CLEAR_VARS)
+
+LOCAL_MODULE:= libnativebridge-dummy
+
+LOCAL_SRC_FILES:= $(NATIVE_BRIDGE_COMMON_SRC_FILES)
+LOCAL_CLANG := true
+LOCAL_CFLAGS += -Werror -Wall
+LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
+LOCAL_LDFLAGS := -ldl
+LOCAL_MULTILIB := both
+
+include $(BUILD_HOST_SHARED_LIBRARY)
diff --git a/libnativebridge/tests/CompleteFlow_test.cpp b/libnativebridge/tests/CompleteFlow_test.cpp
new file mode 100644
index 0000000..4f0fe8b
--- /dev/null
+++ b/libnativebridge/tests/CompleteFlow_test.cpp
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "NativeBridgeTest.h"
+
+namespace android {
+
+TEST_F(NativeBridgeTest, CompleteFlow) {
+ // Init
+ ASSERT_TRUE(LoadNativeBridge(kNativeBridgeLibrary, nullptr));
+ ASSERT_TRUE(NativeBridgeAvailable());
+ ASSERT_TRUE(PreInitializeNativeBridge(".", "isa"));
+ ASSERT_TRUE(NativeBridgeAvailable());
+ ASSERT_TRUE(InitializeNativeBridge(nullptr, nullptr));
+ ASSERT_TRUE(NativeBridgeAvailable());
+
+ // Basic calls to check that nothing crashes
+ ASSERT_FALSE(NativeBridgeIsSupported(nullptr));
+ ASSERT_EQ(nullptr, NativeBridgeLoadLibrary(nullptr, 0));
+ ASSERT_EQ(nullptr, NativeBridgeGetTrampoline(nullptr, nullptr, nullptr, 0));
+
+ // Unload
+ UnloadNativeBridge();
+
+ ASSERT_FALSE(NativeBridgeError());
+}
+
+} // namespace android
diff --git a/libnativebridge/tests/DummyNativeBridge.cpp b/libnativebridge/tests/DummyNativeBridge.cpp
new file mode 100644
index 0000000..b9894f6
--- /dev/null
+++ b/libnativebridge/tests/DummyNativeBridge.cpp
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// A dummy implementation of the native-bridge interface.
+
+#include "nativebridge/native_bridge.h"
+
+// NativeBridgeCallbacks implementations
+extern "C" bool native_bridge_initialize(const android::NativeBridgeRuntimeCallbacks* /* art_cbs */,
+ const char* /* app_code_cache_dir */,
+ const char* /* isa */) {
+ return true;
+}
+
+extern "C" void* native_bridge_loadLibrary(const char* /* libpath */, int /* flag */) {
+ return nullptr;
+}
+
+extern "C" void* native_bridge_getTrampoline(void* /* handle */, const char* /* name */,
+ const char* /* shorty */, uint32_t /* len */) {
+ return nullptr;
+}
+
+extern "C" bool native_bridge_isSupported(const char* /* libpath */) {
+ return false;
+}
+
+extern "C" const struct android::NativeBridgeRuntimeValues* native_bridge_getAppEnv(
+ const char* /* abi */) {
+ return nullptr;
+}
+
+android::NativeBridgeCallbacks NativeBridgeItf {
+ .version = 1,
+ .initialize = &native_bridge_initialize,
+ .loadLibrary = &native_bridge_loadLibrary,
+ .getTrampoline = &native_bridge_getTrampoline,
+ .isSupported = &native_bridge_isSupported,
+ .getAppEnv = &native_bridge_getAppEnv
+};
diff --git a/libnativebridge/tests/NativeBridgeTest.h b/libnativebridge/tests/NativeBridgeTest.h
index 0d731cb..73c92f1 100644
--- a/libnativebridge/tests/NativeBridgeTest.h
+++ b/libnativebridge/tests/NativeBridgeTest.h
@@ -22,6 +22,8 @@
#include <nativebridge/native_bridge.h>
#include <gtest/gtest.h>
+constexpr const char* kNativeBridgeLibrary = "libnativebridge-dummy.so";
+
namespace android {
class NativeBridgeTest : public testing::Test {
diff --git a/libnativebridge/tests/PreInitializeNativeBridgeFail1_test.cpp b/libnativebridge/tests/PreInitializeNativeBridgeFail1_test.cpp
new file mode 100644
index 0000000..69c30a1
--- /dev/null
+++ b/libnativebridge/tests/PreInitializeNativeBridgeFail1_test.cpp
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "NativeBridgeTest.h"
+
+#include <cstdio>
+#include <cstring>
+#include <cutils/log.h>
+#include <dlfcn.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+
+namespace android {
+
+TEST_F(NativeBridgeTest, PreInitializeNativeBridgeFail1) {
+ // Needs a valid application directory.
+ ASSERT_TRUE(LoadNativeBridge(kNativeBridgeLibrary, nullptr));
+ ASSERT_FALSE(PreInitializeNativeBridge(nullptr, "isa"));
+ ASSERT_TRUE(NativeBridgeError());
+}
+
+} // namespace android
diff --git a/libnativebridge/tests/PreInitializeNativeBridgeFail2_test.cpp b/libnativebridge/tests/PreInitializeNativeBridgeFail2_test.cpp
new file mode 100644
index 0000000..74e96e0
--- /dev/null
+++ b/libnativebridge/tests/PreInitializeNativeBridgeFail2_test.cpp
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "NativeBridgeTest.h"
+
+#include <cstdio>
+#include <cstring>
+#include <cutils/log.h>
+#include <dlfcn.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+
+namespace android {
+
+TEST_F(NativeBridgeTest, PreInitializeNativeBridgeFail2) {
+ // Needs LoadNativeBridge() first
+ ASSERT_FALSE(PreInitializeNativeBridge(nullptr, "isa"));
+ ASSERT_TRUE(NativeBridgeError());
+}
+
+} // namespace android
diff --git a/libnativebridge/tests/PreInitializeNativeBridge_test.cpp b/libnativebridge/tests/PreInitializeNativeBridge_test.cpp
index 84078f7..cec26ce 100644
--- a/libnativebridge/tests/PreInitializeNativeBridge_test.cpp
+++ b/libnativebridge/tests/PreInitializeNativeBridge_test.cpp
@@ -31,6 +31,7 @@
static constexpr const char* kTestData = "PreInitializeNativeBridge test.";
TEST_F(NativeBridgeTest, PreInitializeNativeBridge) {
+ ASSERT_TRUE(LoadNativeBridge(kNativeBridgeLibrary, nullptr));
#ifndef __APPLE__ // Mac OS does not support bind-mount.
#ifndef HAVE_ANDROID_OS // Cannot write into the hard-wired location.
// Try to create our mount namespace.
@@ -41,8 +42,7 @@
fprintf(cpuinfo, kTestData);
fclose(cpuinfo);
- // Call the setup.
- PreInitializeNativeBridge("does not matter 1", "short 2");
+ ASSERT_TRUE(PreInitializeNativeBridge("does not matter 1", "short 2"));
// Read /proc/cpuinfo
FILE* proc_cpuinfo = fopen("/proc/cpuinfo", "r");