Return error message if IndirectReferenceTable construction fails.
Previously if there was an error when constructing the
IndirectReferenceTable, the error message was lost. Now expose and
include the error message when throwing an exception related to
failures to construct the IndirectReferenceTable.
The error message is propagated through JVMEnvExt, JavaVMExt, and
Runtime::Init as well.
Bug: 32013594
Test: Added new 151-OpenFileLimit runtest.
Test: m test-art-host, m test-art-target
Change-Id: I3692f6928c9570358571bce634569d6f14cdeb05
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 99edb7c..eec1903 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 39f4d97..c9bd14e 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 9622a8e..670cc3e 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 6e15c38..bde4185 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 6acce27..fac3ac4 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);
+ }
+ }
+ }
+}
