diff options
| -rw-r--r-- | runtime/indirect_reference_table.cc | 25 | ||||
| -rw-r--r-- | runtime/indirect_reference_table.h | 18 | ||||
| -rw-r--r-- | runtime/indirect_reference_table_test.cc | 4 | ||||
| -rw-r--r-- | runtime/java_vm_ext.cc | 21 | ||||
| -rw-r--r-- | runtime/java_vm_ext.h | 13 | ||||
| -rw-r--r-- | runtime/jni_env_ext.cc | 8 | ||||
| -rw-r--r-- | runtime/jni_env_ext.h | 8 | ||||
| -rw-r--r-- | runtime/jni_internal_test.cc | 4 | ||||
| -rw-r--r-- | runtime/runtime.cc | 11 | ||||
| -rw-r--r-- | runtime/runtime.h | 4 | ||||
| -rw-r--r-- | runtime/thread.cc | 9 | ||||
| -rw-r--r-- | test/151-OpenFileLimit/expected.txt | 3 | ||||
| -rw-r--r-- | test/151-OpenFileLimit/info.txt | 3 | ||||
| -rw-r--r-- | test/151-OpenFileLimit/src/Main.java | 82 |
14 files changed, 174 insertions, 39 deletions
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 130c10d322..7389c73096 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -60,27 +60,24 @@ void IndirectReferenceTable::AbortIfNoCheckJNI(const std::string& msg) { 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"; + } + + if (table_mem_map_.get() != nullptr) { + table_ = reinterpret_cast<IrtEntry*>(table_mem_map_->Begin()); + } else { + table_ = nullptr; } - table_ = reinterpret_cast<IrtEntry*>(table_mem_map_->Begin()); segment_state_.all = IRT_FIRST_SEGMENT; } diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index 1762b10350..363280a87c 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -257,12 +257,24 @@ bool inline operator!=(const IrtIterator& lhs, const IrtIterator& rhs) { 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 7b28f0bda8..d7026de559 100644 --- a/runtime/indirect_reference_table_test.cc +++ b/runtime/indirect_reference_table_test.cc @@ -49,7 +49,9 @@ TEST_F(IndirectReferenceTableTest, BasicTest) { 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 7285b9a965..9b4327f137 100644 --- a/runtime/java_vm_ext.cc +++ b/runtime/java_vm_ext.cc @@ -411,7 +411,9 @@ const JNIInvokeInterface gJniInvokeInterface = { 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 @@ JavaVMExt::JavaVMExt(Runtime* runtime, const RuntimeArgumentMap& runtime_options 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(Runtime* runtime, const RuntimeArgumentMap& runtime_options 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 05717f41e7..9e37f1178c 100644 --- a/runtime/java_vm_ext.h +++ b/runtime/java_vm_ext.h @@ -43,7 +43,14 @@ using GetEnvHook = jint (*)(JavaVMExt* vm, /*out*/void** new_env, jint version); 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 @@ class JavaVMExt : public JavaVM { 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 1ca2cb4146..8eca8fcba9 100644 --- a/runtime/jni_env_ext.cc +++ b/runtime/jni_env_ext.cc @@ -57,19 +57,19 @@ jint JNIEnvExt::GetEnvHandler(JavaVMExt* vm, /*out*/void** env, jint version) { 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 549f8c56a0..e89debbf90 100644 --- a/runtime/jni_env_ext.h +++ b/runtime/jni_env_ext.h @@ -34,7 +34,9 @@ class JavaVMExt; 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 @@ struct JNIEnvExt : public JNIEnv { 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 c6d5c9ea61..9479a181c6 100644 --- a/runtime/jni_internal_test.cc +++ b/runtime/jni_internal_test.cc @@ -2307,7 +2307,9 @@ TEST_F(JniInternalTest, IndirectReferenceTableOffsets) { // 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 6e15c38a53..bde41858cf 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -347,13 +347,13 @@ Runtime::~Runtime() { 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 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { } } - 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 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { 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 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { &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 e2ba2626e9..7cb87abe30 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -277,7 +277,7 @@ class Runtime { } JavaVMExt* GetJavaVM() const { - return java_vm_; + return java_vm_.get(); } size_t GetMaxSpinsBeforeThinkLockInflation() const { @@ -757,7 +757,7 @@ class Runtime { 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 39fe8d09c1..e47ccc062b 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -606,8 +606,9 @@ void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_siz // 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 @@ void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_siz 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 @@ bool Thread::Init(ThreadList* thread_list, JavaVMExt* java_vm, JNIEnvExt* jni_en 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 0000000000..971e472bff --- /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 0000000000..56ed3963f4 --- /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 0000000000..01a9a4ed34 --- /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); + } + } + } +} |