Merge "Return error message if IndirectReferenceTable construction fails."
am: 3bbfcb794f
Change-Id: Ib7f88016ae51ce543542ada3dd0c578d0a463c22
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 130c10d..7389c73 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -60,27 +60,24 @@
IndirectReferenceTable::IndirectReferenceTable(size_t max_count,
IndirectRefKind desired_kind,
- bool abort_on_error)
+ std::string* error_msg)
: kind_(desired_kind),
max_entries_(max_count) {
+ CHECK(error_msg != nullptr);
CHECK_NE(desired_kind, kHandleScopeOrInvalid);
- std::string error_str;
const size_t table_bytes = max_count * sizeof(IrtEntry);
table_mem_map_.reset(MemMap::MapAnonymous("indirect ref table", nullptr, table_bytes,
- PROT_READ | PROT_WRITE, false, false, &error_str));
- if (abort_on_error) {
- CHECK(table_mem_map_.get() != nullptr) << error_str;
- CHECK_EQ(table_mem_map_->Size(), table_bytes);
- CHECK(table_mem_map_->Begin() != nullptr);
- } else if (table_mem_map_.get() == nullptr ||
- table_mem_map_->Size() != table_bytes ||
- table_mem_map_->Begin() == nullptr) {
- table_mem_map_.reset();
- LOG(ERROR) << error_str;
- return;
+ PROT_READ | PROT_WRITE, false, false, error_msg));
+ if (table_mem_map_.get() == nullptr && error_msg->empty()) {
+ *error_msg = "Unable to map memory for indirect ref table";
}
- table_ = reinterpret_cast<IrtEntry*>(table_mem_map_->Begin());
+
+ if (table_mem_map_.get() != nullptr) {
+ table_ = reinterpret_cast<IrtEntry*>(table_mem_map_->Begin());
+ } else {
+ table_ = nullptr;
+ }
segment_state_.all = IRT_FIRST_SEGMENT;
}
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index 1762b10..363280a 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -257,12 +257,24 @@
class IndirectReferenceTable {
public:
- // WARNING: When using with abort_on_error = false, the object may be in a partially
- // initialized state. Use IsValid() to check.
- IndirectReferenceTable(size_t max_count, IndirectRefKind kind, bool abort_on_error = true);
+ /*
+ * WARNING: Construction of the IndirectReferenceTable may fail.
+ * error_msg must not be null. If error_msg is set by the constructor, then
+ * construction has failed and the IndirectReferenceTable will be in an
+ * invalid state. Use IsValid to check whether the object is in an invalid
+ * state.
+ */
+ IndirectReferenceTable(size_t max_count, IndirectRefKind kind, std::string* error_msg);
~IndirectReferenceTable();
+ /*
+ * Checks whether construction of the IndirectReferenceTable succeeded.
+ *
+ * This object must only be used if IsValid() returns true. It is safe to
+ * call IsValid from multiple threads without locking or other explicit
+ * synchronization.
+ */
bool IsValid() const;
/*
diff --git a/runtime/indirect_reference_table_test.cc b/runtime/indirect_reference_table_test.cc
index 7b28f0b..d7026de 100644
--- a/runtime/indirect_reference_table_test.cc
+++ b/runtime/indirect_reference_table_test.cc
@@ -49,7 +49,9 @@
ScopedObjectAccess soa(Thread::Current());
static const size_t kTableMax = 20;
- IndirectReferenceTable irt(kTableMax, kGlobal);
+ std::string error_msg;
+ IndirectReferenceTable irt(kTableMax, kGlobal, &error_msg);
+ ASSERT_TRUE(irt.IsValid()) << error_msg;
mirror::Class* c = class_linker_->FindSystemClass(soa.Self(), "Ljava/lang/Object;");
StackHandleScope<4> hs(soa.Self());
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index 7285b9a..9b4327f 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -411,7 +411,9 @@
JII::AttachCurrentThreadAsDaemon
};
-JavaVMExt::JavaVMExt(Runtime* runtime, const RuntimeArgumentMap& runtime_options)
+JavaVMExt::JavaVMExt(Runtime* runtime,
+ const RuntimeArgumentMap& runtime_options,
+ std::string* error_msg)
: runtime_(runtime),
check_jni_abort_hook_(nullptr),
check_jni_abort_hook_data_(nullptr),
@@ -420,10 +422,10 @@
tracing_enabled_(runtime_options.Exists(RuntimeArgumentMap::JniTrace)
|| VLOG_IS_ON(third_party_jni)),
trace_(runtime_options.GetOrDefault(RuntimeArgumentMap::JniTrace)),
- globals_(kGlobalsMax, kGlobal),
+ globals_(kGlobalsMax, kGlobal, error_msg),
libraries_(new Libraries),
unchecked_functions_(&gJniInvokeInterface),
- weak_globals_(kWeakGlobalsMax, kWeakGlobal),
+ weak_globals_(kWeakGlobalsMax, kWeakGlobal, error_msg),
allow_accessing_weak_globals_(true),
weak_globals_add_condition_("weak globals add condition",
(CHECK(Locks::jni_weak_globals_lock_ != nullptr),
@@ -436,6 +438,19 @@
JavaVMExt::~JavaVMExt() {
}
+// Checking "globals" and "weak_globals" usually requires locks, but we
+// don't need the locks to check for validity when constructing the
+// object. Use NO_THREAD_SAFETY_ANALYSIS for this.
+std::unique_ptr<JavaVMExt> JavaVMExt::Create(Runtime* runtime,
+ const RuntimeArgumentMap& runtime_options,
+ std::string* error_msg) NO_THREAD_SAFETY_ANALYSIS {
+ std::unique_ptr<JavaVMExt> java_vm(new JavaVMExt(runtime, runtime_options, error_msg));
+ if (java_vm && java_vm->globals_.IsValid() && java_vm->weak_globals_.IsValid()) {
+ return java_vm;
+ }
+ return nullptr;
+}
+
jint JavaVMExt::HandleGetEnv(/*out*/void** env, jint version) {
for (GetEnvHook hook : env_hooks_) {
jint res = hook(this, env, version);
diff --git a/runtime/java_vm_ext.h b/runtime/java_vm_ext.h
index 05717f4..9e37f11 100644
--- a/runtime/java_vm_ext.h
+++ b/runtime/java_vm_ext.h
@@ -43,7 +43,14 @@
class JavaVMExt : public JavaVM {
public:
- JavaVMExt(Runtime* runtime, const RuntimeArgumentMap& runtime_options);
+ // Creates a new JavaVMExt object.
+ // Returns nullptr on error, in which case error_msg is set to a message
+ // describing the error.
+ static std::unique_ptr<JavaVMExt> Create(Runtime* runtime,
+ const RuntimeArgumentMap& runtime_options,
+ std::string* error_msg);
+
+
~JavaVMExt();
bool ForceCopy() const {
@@ -192,6 +199,10 @@
static bool IsBadJniVersion(int version);
private:
+ // The constructor should not be called directly. It may leave the object in
+ // an erroneous state, and the result needs to be checked.
+ JavaVMExt(Runtime* runtime, const RuntimeArgumentMap& runtime_options, std::string* error_msg);
+
// Return true if self can currently access weak globals.
bool MayAccessWeakGlobalsUnlocked(Thread* self) const REQUIRES_SHARED(Locks::mutator_lock_);
bool MayAccessWeakGlobals(Thread* self) const
diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc
index 1ca2cb4..8eca8fc 100644
--- a/runtime/jni_env_ext.cc
+++ b/runtime/jni_env_ext.cc
@@ -57,19 +57,19 @@
return JNI_OK;
}
-JNIEnvExt* JNIEnvExt::Create(Thread* self_in, JavaVMExt* vm_in) {
- std::unique_ptr<JNIEnvExt> ret(new JNIEnvExt(self_in, vm_in));
+JNIEnvExt* JNIEnvExt::Create(Thread* self_in, JavaVMExt* vm_in, std::string* error_msg) {
+ std::unique_ptr<JNIEnvExt> ret(new JNIEnvExt(self_in, vm_in, error_msg));
if (CheckLocalsValid(ret.get())) {
return ret.release();
}
return nullptr;
}
-JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in)
+JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in, std::string* error_msg)
: self(self_in),
vm(vm_in),
local_ref_cookie(IRT_FIRST_SEGMENT),
- locals(kLocalsInitial, kLocal, false),
+ locals(kLocalsInitial, kLocal, error_msg),
check_jni(false),
runtime_deleted(false),
critical(0),
diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h
index 549f8c5..e89debb 100644
--- a/runtime/jni_env_ext.h
+++ b/runtime/jni_env_ext.h
@@ -34,7 +34,9 @@
static constexpr size_t kLocalsInitial = 512;
struct JNIEnvExt : public JNIEnv {
- static JNIEnvExt* Create(Thread* self, JavaVMExt* vm);
+ // Creates a new JNIEnvExt. Returns null on error, in which case error_msg
+ // will contain a description of the error.
+ static JNIEnvExt* Create(Thread* self, JavaVMExt* vm, std::string* error_msg);
~JNIEnvExt();
@@ -103,9 +105,9 @@
void SetFunctionsToRuntimeShutdownFunctions();
private:
- // The constructor should not be called directly. It may leave the object in an erronuous state,
+ // The constructor should not be called directly. It may leave the object in an erroneous state,
// and the result needs to be checked.
- JNIEnvExt(Thread* self, JavaVMExt* vm);
+ JNIEnvExt(Thread* self, JavaVMExt* vm, std::string* error_msg);
// All locked objects, with the (Java caller) stack frame that locked them. Used in CheckJNI
// to ensure that only monitors locked in this native frame are being unlocked, and that at
diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc
index c6d5c9e..9479a18 100644
--- a/runtime/jni_internal_test.cc
+++ b/runtime/jni_internal_test.cc
@@ -2307,7 +2307,9 @@
// The segment_state_ field is private, and we want to avoid friend declaration. So we'll check
// by modifying memory.
// The parameters don't really matter here.
- IndirectReferenceTable irt(5, IndirectRefKind::kGlobal, true);
+ std::string error_msg;
+ IndirectReferenceTable irt(5, IndirectRefKind::kGlobal, &error_msg);
+ ASSERT_TRUE(irt.IsValid()) << error_msg;
uint32_t old_state = irt.GetSegmentState();
// Write some new state directly. We invert parts of old_state to ensure a new value.
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index d3bba24..b7c1cdd 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -347,13 +347,13 @@
delete class_linker_;
delete heap_;
delete intern_table_;
- delete java_vm_;
delete oat_file_manager_;
Thread::Shutdown();
QuasiAtomic::Shutdown();
verifier::MethodVerifier::Shutdown();
// Destroy allocators before shutting down the MemMap because they may use it.
+ java_vm_.reset();
linear_alloc_.reset();
low_4gb_arena_pool_.reset();
arena_pool_.reset();
@@ -1120,7 +1120,12 @@
}
}
- java_vm_ = new JavaVMExt(this, runtime_options);
+ std::string error_msg;
+ java_vm_ = JavaVMExt::Create(this, runtime_options, &error_msg);
+ if (java_vm_.get() == nullptr) {
+ LOG(ERROR) << "Could not initialize JavaVMExt: " << error_msg;
+ return false;
+ }
// Add the JniEnv handler.
// TODO Refactor this stuff.
@@ -1144,7 +1149,6 @@
CHECK_GE(GetHeap()->GetContinuousSpaces().size(), 1U);
class_linker_ = new ClassLinker(intern_table_);
if (GetHeap()->HasBootImageSpace()) {
- std::string error_msg;
bool result = class_linker_->InitFromBootImage(&error_msg);
if (!result) {
LOG(ERROR) << "Could not initialize from image: " << error_msg;
@@ -1191,7 +1195,6 @@
&boot_class_path);
}
instruction_set_ = runtime_options.GetOrDefault(Opt::ImageInstructionSet);
- std::string error_msg;
if (!class_linker_->InitWithoutImage(std::move(boot_class_path), &error_msg)) {
LOG(ERROR) << "Could not initialize without image: " << error_msg;
return false;
diff --git a/runtime/runtime.h b/runtime/runtime.h
index e2ba262..7cb87ab 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -277,7 +277,7 @@
}
JavaVMExt* GetJavaVM() const {
- return java_vm_;
+ return java_vm_.get();
}
size_t GetMaxSpinsBeforeThinkLockInflation() const {
@@ -757,7 +757,7 @@
SignalCatcher* signal_catcher_;
std::string stack_trace_file_;
- JavaVMExt* java_vm_;
+ std::unique_ptr<JavaVMExt> java_vm_;
std::unique_ptr<jit::Jit> jit_;
std::unique_ptr<jit::JitOptions> jit_options_;
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 39fe8d0..e47ccc0 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -606,8 +606,9 @@
// Try to allocate a JNIEnvExt for the thread. We do this here as we might be out of memory and
// do not have a good way to report this on the child's side.
+ std::string error_msg;
std::unique_ptr<JNIEnvExt> child_jni_env_ext(
- JNIEnvExt::Create(child_thread, Runtime::Current()->GetJavaVM()));
+ JNIEnvExt::Create(child_thread, Runtime::Current()->GetJavaVM(), &error_msg));
int pthread_create_result = 0;
if (child_jni_env_ext.get() != nullptr) {
@@ -648,7 +649,7 @@
env->SetLongField(java_peer, WellKnownClasses::java_lang_Thread_nativePeer, 0);
{
std::string msg(child_jni_env_ext.get() == nullptr ?
- "Could not allocate JNI Env" :
+ StringPrintf("Could not allocate JNI Env: %s", error_msg.c_str()) :
StringPrintf("pthread_create (%s stack) failed: %s",
PrettySize(stack_size).c_str(), strerror(pthread_create_result)));
ScopedObjectAccess soa(env);
@@ -693,8 +694,10 @@
DCHECK_EQ(jni_env_ext->self, this);
tlsPtr_.jni_env = jni_env_ext;
} else {
- tlsPtr_.jni_env = JNIEnvExt::Create(this, java_vm);
+ std::string error_msg;
+ tlsPtr_.jni_env = JNIEnvExt::Create(this, java_vm, &error_msg);
if (tlsPtr_.jni_env == nullptr) {
+ LOG(ERROR) << "Failed to create JNIEnvExt: " << error_msg;
return false;
}
}
diff --git a/test/151-OpenFileLimit/expected.txt b/test/151-OpenFileLimit/expected.txt
new file mode 100644
index 0000000..971e472
--- /dev/null
+++ b/test/151-OpenFileLimit/expected.txt
@@ -0,0 +1,3 @@
+Message includes "Too many open files"
+Message includes "Too many open files"
+done.
diff --git a/test/151-OpenFileLimit/info.txt b/test/151-OpenFileLimit/info.txt
new file mode 100644
index 0000000..56ed396
--- /dev/null
+++ b/test/151-OpenFileLimit/info.txt
@@ -0,0 +1,3 @@
+This test verifies the exception message is informative for failure to launch
+a thread due to the number of available file descriptors in the process being
+exceeded.
diff --git a/test/151-OpenFileLimit/src/Main.java b/test/151-OpenFileLimit/src/Main.java
new file mode 100644
index 0000000..01a9a4e
--- /dev/null
+++ b/test/151-OpenFileLimit/src/Main.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+import static java.nio.file.StandardOpenOption.*;
+import java.nio.file.*;
+import java.io.*;
+import java.util.*;
+
+public class Main {
+ private static final String TEMP_FILE_NAME_PREFIX = "oflimit";
+ private static final String TEMP_FILE_NAME_SUFFIX = ".txt";
+
+ public static void main(String[] args) throws IOException {
+
+ // Exhaust the number of open file descriptors.
+ List<File> files = new ArrayList<File>();
+ List<OutputStream> streams = new ArrayList<OutputStream>();
+ try {
+ for (int i = 0; ; i++) {
+ File file = createTempFile();
+ files.add(file);
+ streams.add(Files.newOutputStream(file.toPath(), CREATE, APPEND));
+ }
+ } catch (Throwable e) {
+ if (e.getMessage().contains("Too many open files")) {
+ System.out.println("Message includes \"Too many open files\"");
+ } else {
+ System.out.println(e.getMessage());
+ }
+ }
+
+ // Now try to create a new thread.
+ try {
+ Thread thread = new Thread() {
+ public void run() {
+ System.out.println("thread run.");
+ }
+ };
+ thread.start();
+ thread.join();
+ } catch (Throwable e) {
+ if (e.getMessage().contains("Too many open files")) {
+ System.out.println("Message includes \"Too many open files\"");
+ } else {
+ System.out.println(e.getMessage());
+ }
+ }
+
+ for (int i = 0; i < files.size(); i++) {
+ streams.get(i).close();
+ files.get(i).delete();
+ }
+ System.out.println("done.");
+ }
+
+ private static File createTempFile() throws Exception {
+ try {
+ return File.createTempFile(TEMP_FILE_NAME_PREFIX, TEMP_FILE_NAME_SUFFIX);
+ } catch (IOException e) {
+ System.setProperty("java.io.tmpdir", "/data/local/tmp");
+ try {
+ return File.createTempFile(TEMP_FILE_NAME_PREFIX, TEMP_FILE_NAME_SUFFIX);
+ } catch (IOException e2) {
+ System.setProperty("java.io.tmpdir", "/sdcard");
+ return File.createTempFile(TEMP_FILE_NAME_PREFIX, TEMP_FILE_NAME_SUFFIX);
+ }
+ }
+ }
+}