From cc07760776c3d2fb2ebc90858b6c20c48c78d783 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Thu, 17 Sep 2015 14:03:21 -0700 Subject: ART: Clean up less in dex2oat In non-debug builds, clean up even less. We already did not shut down the runtime. Also skipping the compiler driver and the verification results removes all major points of destructor performance. Tested with a common large app on Nexus 9. Time between dex2oat timing message and executable exit (log from immediately-after log echo) [w/o swap, w/ swap]. Before: 2.409s / 48.774s After: 0.132s / 0.188s Bug: 24199200 Change-Id: I5d8c17f8e28796545cfbb3887c07c92905f9b48d (cherry picked from commit 3f30e1219dd76f78bb9b6504e696a04a3dfae564) --- dex2oat/dex2oat.cc | 57 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index e913c20..466f5e6 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -502,6 +502,7 @@ class Dex2Oat FINAL { compiler_kind_(kUseOptimizingCompiler ? Compiler::kOptimizing : Compiler::kQuick), instruction_set_(kRuntimeISA), // Take the default set of instruction features from the build. + verification_results_(nullptr), method_inliner_map_(), runtime_(nullptr), thread_count_(sysconf(_SC_NPROCESSORS_CONF)), @@ -517,6 +518,7 @@ class Dex2Oat FINAL { compiled_methods_filename_(nullptr), image_(false), is_host_(false), + driver_(nullptr), dump_stats_(false), dump_passes_(false), dump_timing_(false), @@ -536,6 +538,8 @@ class Dex2Oat FINAL { if (kIsDebugBuild || (RUNNING_ON_VALGRIND != 0)) { delete runtime_; // See field declaration for why this is manual. + delete driver_; + delete verification_results_; } } @@ -1167,9 +1171,9 @@ class Dex2Oat FINAL { runtime_options.push_back(std::make_pair(runtime_args_[i], nullptr)); } - verification_results_.reset(new VerificationResults(compiler_options_.get())); + verification_results_ = new VerificationResults(compiler_options_.get()); callbacks_.reset(new QuickCompilerCallbacks( - verification_results_.get(), + verification_results_, &method_inliner_map_, image_ ? CompilerCallbacks::CallbackMode::kCompileBootImage : @@ -1381,23 +1385,23 @@ class Dex2Oat FINAL { class_loader = class_linker->CreatePathClassLoader(self, class_path_files); } - driver_.reset(new CompilerDriver(compiler_options_.get(), - verification_results_.get(), - &method_inliner_map_, - compiler_kind_, - instruction_set_, - instruction_set_features_.get(), - image_, - image_classes_.release(), - compiled_classes_.release(), - nullptr, - thread_count_, - dump_stats_, - dump_passes_, - dump_cfg_file_name_, - compiler_phases_timings_.get(), - swap_fd_, - profile_file_)); + driver_ = new CompilerDriver(compiler_options_.get(), + verification_results_, + &method_inliner_map_, + compiler_kind_, + instruction_set_, + instruction_set_features_.get(), + image_, + image_classes_.release(), + compiled_classes_.release(), + nullptr, + thread_count_, + dump_stats_, + dump_passes_, + dump_cfg_file_name_, + compiler_phases_timings_.get(), + swap_fd_, + profile_file_); driver_->CompileAll(class_loader, dex_files_, timings_); } @@ -1499,7 +1503,7 @@ class Dex2Oat FINAL { oat_writer.reset(new OatWriter(dex_files_, image_file_location_oat_checksum, image_file_location_oat_data_begin, image_patch_delta, - driver_.get(), + driver_, image_writer_.get(), timings_, key_value_store_.get())); @@ -1839,7 +1843,7 @@ class Dex2Oat FINAL { // Note: driver creation can fail when loading an invalid dex file. LOG(INFO) << "dex2oat took " << PrettyDuration(NanoTime() - start_ns_) << " (threads: " << thread_count_ << ") " - << ((Runtime::Current() != nullptr && driver_.get() != nullptr) ? + << ((Runtime::Current() != nullptr && driver_ != nullptr) ? driver_->GetMemoryUsageString(kIsDebugBuild || VLOG_IS_ON(compiler)) : ""); } @@ -1852,7 +1856,10 @@ class Dex2Oat FINAL { std::unique_ptr > key_value_store_; - std::unique_ptr verification_results_; + // Not a unique_ptr as we want to just exit on non-debug builds, not bringing the compiler down + // in an orderly fashion. The destructor takes care of deleting this. + VerificationResults* verification_results_; + DexFileToMethodInlinerMap method_inliner_map_; std::unique_ptr callbacks_; @@ -1895,7 +1902,11 @@ class Dex2Oat FINAL { std::string android_root_; std::vector dex_files_; std::vector> opened_dex_files_; - std::unique_ptr driver_; + + // Not a unique_ptr as we want to just exit on non-debug builds, not bringing the driver down + // in an orderly fashion. The destructor takes care of deleting this. + CompilerDriver* driver_; + std::vector verbose_methods_; bool dump_stats_; bool dump_passes_; -- cgit v1.1 From 2a196784553f4fd0c0f7d4b8aac87281db3a4748 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Thu, 1 Oct 2015 16:47:26 -0700 Subject: ART: Do not abort on exception in CreatePeer Different parts of CreatePeer may throw an exception, especially the Thread constructor. Do not abort in such a case, but return and report a failure to attach/create a thread. Bug: 24200698 Change-Id: I06f2c997f0451c71f791d1f12bea6f8ee65e8ab2 --- runtime/thread.cc | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/runtime/thread.cc b/runtime/thread.cc index 5274f9e..6e8f89c 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -546,6 +546,18 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_g // a native peer! if (create_peer) { self->CreatePeer(thread_name, as_daemon, thread_group); + if (self->IsExceptionPending()) { + // We cannot keep the exception around, as we're deleting self. Try to be helpful and log it. + { + ScopedObjectAccess soa(self); + LOG(ERROR) << "Exception creating thread peer:"; + LOG(ERROR) << self->GetException()->Dump(); + self->ClearException(); + } + runtime->GetThreadList()->Unregister(self); + // Unregister deletes self, no need to do this here. + return nullptr; + } } else { // These aren't necessary, but they improve diagnostics for unit tests & command-line tools. if (thread_name != nullptr) { @@ -594,7 +606,9 @@ void Thread::CreatePeer(const char* name, bool as_daemon, jobject thread_group) WellKnownClasses::java_lang_Thread, WellKnownClasses::java_lang_Thread_init, thread_group, thread_name.get(), thread_priority, thread_is_daemon); - AssertNoPendingException(); + if (IsExceptionPending()) { + return; + } Thread* self = this; DCHECK_EQ(self, Thread::Current()); @@ -1256,6 +1270,7 @@ void Thread::FinishStartup() { // Finish attaching the main thread. ScopedObjectAccess soa(Thread::Current()); Thread::Current()->CreatePeer("main", false, runtime->GetMainThreadGroup()); + Thread::Current()->AssertNoPendingException(); Runtime::Current()->GetClassLinker()->RunRootClinits(); } -- cgit v1.1 From 22e0ce3a73760757e509032a324bbbb9a1a93f5e Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 8 Oct 2015 15:17:15 -0700 Subject: DO NOT MERGE Add locking to prevent races between setting class methods and marking There was a race condition between VisitNativeRoots and threads which were updating the lengths and pointers of the direct or virtual methods of classes. For example: The thread doing VisitNativeRoots could see a null pointer with a non 0 length if another thread had changed the length but not the pointer. The fix is already in master, do not merge. Bug: 24270063 Change-Id: Id7280b9507b95703820aedb6c5fee49966dabe27 --- runtime/class_linker.cc | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 9ca6492..d0e8e68 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -2334,15 +2334,22 @@ void ClassLinker::LoadClassMembers(Thread* self, const DexFile& dex_file, klass->SetIFields(ifields); klass->SetNumInstanceFields(num_ifields); DCHECK_EQ(klass->NumInstanceFields(), num_ifields); - // Load methods. - if (it.NumDirectMethods() != 0) { - klass->SetDirectMethodsPtr(AllocArtMethodArray(self, it.NumDirectMethods())); - } - klass->SetNumDirectMethods(it.NumDirectMethods()); - if (it.NumVirtualMethods() != 0) { - klass->SetVirtualMethodsPtr(AllocArtMethodArray(self, it.NumVirtualMethods())); + ArtMethod* const direct_methods = (it.NumDirectMethods() != 0) + ? AllocArtMethodArray(self, it.NumDirectMethods()) + : nullptr; + ArtMethod* const virtual_methods = (it.NumVirtualMethods() != 0) + ? AllocArtMethodArray(self, it.NumVirtualMethods()) + : nullptr; + { + // Used to get exclusion between with VisitNativeRoots so that no thread sees a length for + // one array with a pointer for a different array. + WriterMutexLock mu(self, *Locks::classlinker_classes_lock_); + // Load methods. + klass->SetDirectMethodsPtr(direct_methods); + klass->SetNumDirectMethods(it.NumDirectMethods()); + klass->SetVirtualMethodsPtr(virtual_methods); + klass->SetNumVirtualMethods(it.NumVirtualMethods()); } - klass->SetNumVirtualMethods(it.NumVirtualMethods()); size_t class_def_method_index = 0; uint32_t last_dex_method_index = DexFile::kDexNoIndex; size_t last_class_def_method_index = 0; @@ -3321,8 +3328,11 @@ mirror::Class* ClassLinker::CreateProxyClass(ScopedObjectAccessAlreadyRunnable& self->AssertPendingOOMException(); return nullptr; } - klass->SetDirectMethodsPtr(directs); - klass->SetNumDirectMethods(1u); + { + WriterMutexLock mu(self, *Locks::classlinker_classes_lock_); + klass->SetDirectMethodsPtr(directs); + klass->SetNumDirectMethods(1u); + } CreateProxyConstructor(klass, klass->GetDirectMethodUnchecked(0, image_pointer_size_)); // Create virtual method using specified prototypes. @@ -3337,8 +3347,11 @@ mirror::Class* ClassLinker::CreateProxyClass(ScopedObjectAccessAlreadyRunnable& self->AssertPendingOOMException(); return nullptr; } - klass->SetVirtualMethodsPtr(virtuals); - klass->SetNumVirtualMethods(num_virtual_methods); + { + WriterMutexLock mu(self, *Locks::classlinker_classes_lock_); + klass->SetVirtualMethodsPtr(virtuals); + klass->SetNumVirtualMethods(num_virtual_methods); + } for (size_t i = 0; i < num_virtual_methods; ++i) { auto* virtual_method = klass->GetVirtualMethodUnchecked(i, image_pointer_size_); auto* prototype = h_methods->Get(i)->GetArtMethod(); -- cgit v1.1 From 2602aa86d8e0e98f00311ea05395548b4d327f82 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 14 Sep 2015 15:34:38 -0700 Subject: ART: Decrease dex2oat watchdog timeout Keep the dex2oat watchdog timeout lower than the package manager timeout, so that dex2oat kills itself before the system server watchdog kills the system because of the long installation. Bug: 23629410 (cherry picked from commit 540138ae55ac1909606a436d7f52e20146c56657) Change-Id: I425b19ab305cfaa43f6bddc3a892be892acaf513 --- dex2oat/dex2oat.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 466f5e6..9d7e68a 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -433,8 +433,10 @@ class WatchDog { // Debug builds are slower so they have larger timeouts. static constexpr int64_t kSlowdownFactor = kIsDebugBuild ? 5U : 1U; - // 10 minutes scaled by kSlowdownFactor. - static constexpr int64_t kWatchDogTimeoutSeconds = kSlowdownFactor * 10 * 60; + // 9.5 minutes scaled by kSlowdownFactor. This is slightly smaller than the Package Manager + // watchdog (PackageManagerService.WATCHDOG_TIMEOUT, 10 minutes), so that dex2oat will abort + // itself before that watchdog would take down the system server. + static constexpr int64_t kWatchDogTimeoutSeconds = kSlowdownFactor * (9 * 60 + 30); bool is_watch_dog_enabled_; bool shutting_down_; -- cgit v1.1 From f015c2f4835a9985a8b31cfc810129b69865a6c4 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 8 Sep 2015 17:42:59 -0700 Subject: ART: Add missing GetInterfaceMethodIfProxy Add missing uses of GetInterfaceMethodIfProxy in reflection code. Add a test case for a JNI call to a proxy method. Bug: https://code.google.com/p/android-developer-preview/issues/detail?id=2973 Bug: 23886441 (cherry picked from commit e80673245c0433a71a4930e460be5dc0920885b2) Change-Id: I5b66b64b5561fcee15d0314707d67e8abc02ce5b --- runtime/reflection.cc | 8 ++--- test/044-proxy/expected.txt | 1 + test/044-proxy/native_proxy.cc | 32 +++++++++++++++++++ test/044-proxy/src/Main.java | 1 + test/044-proxy/src/NativeProxy.java | 62 +++++++++++++++++++++++++++++++++++++ test/Android.libarttest.mk | 1 + 6 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 test/044-proxy/native_proxy.cc create mode 100644 test/044-proxy/src/NativeProxy.java diff --git a/runtime/reflection.cc b/runtime/reflection.cc index 11522d9..db09afb 100644 --- a/runtime/reflection.cc +++ b/runtime/reflection.cc @@ -453,7 +453,7 @@ JValue InvokeWithVarArgs(const ScopedObjectAccessAlreadyRunnable& soa, jobject o } mirror::Object* receiver = method->IsStatic() ? nullptr : soa.Decode(obj); uint32_t shorty_len = 0; - const char* shorty = method->GetShorty(&shorty_len); + const char* shorty = method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty(&shorty_len); JValue result; ArgArray arg_array(shorty, shorty_len); arg_array.BuildArgArrayFromVarArgs(soa, receiver, args); @@ -483,7 +483,7 @@ JValue InvokeWithJValues(const ScopedObjectAccessAlreadyRunnable& soa, jobject o } mirror::Object* receiver = method->IsStatic() ? nullptr : soa.Decode(obj); uint32_t shorty_len = 0; - const char* shorty = method->GetShorty(&shorty_len); + const char* shorty = method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty(&shorty_len); JValue result; ArgArray arg_array(shorty, shorty_len); arg_array.BuildArgArrayFromJValues(soa, receiver, args); @@ -514,7 +514,7 @@ JValue InvokeVirtualOrInterfaceWithJValues(const ScopedObjectAccessAlreadyRunnab receiver = nullptr; } uint32_t shorty_len = 0; - const char* shorty = method->GetShorty(&shorty_len); + const char* shorty = method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty(&shorty_len); JValue result; ArgArray arg_array(shorty, shorty_len); arg_array.BuildArgArrayFromJValues(soa, receiver, args); @@ -545,7 +545,7 @@ JValue InvokeVirtualOrInterfaceWithVarArgs(const ScopedObjectAccessAlreadyRunnab receiver = nullptr; } uint32_t shorty_len = 0; - const char* shorty = method->GetShorty(&shorty_len); + const char* shorty = method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty(&shorty_len); JValue result; ArgArray arg_array(shorty, shorty_len); arg_array.BuildArgArrayFromVarArgs(soa, receiver, args); diff --git a/test/044-proxy/expected.txt b/test/044-proxy/expected.txt index bcce019..f86948a 100644 --- a/test/044-proxy/expected.txt +++ b/test/044-proxy/expected.txt @@ -93,3 +93,4 @@ Invocation of public abstract java.lang.String NarrowingTest$I2.foo() Got expected exception Proxy narrowed invocation return type passed 5.8 +callback diff --git a/test/044-proxy/native_proxy.cc b/test/044-proxy/native_proxy.cc new file mode 100644 index 0000000..f168719 --- /dev/null +++ b/test/044-proxy/native_proxy.cc @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2015 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 "jni.h" + +#include "base/logging.h" + +namespace art { + +extern "C" JNIEXPORT void JNICALL Java_NativeProxy_nativeCall( + JNIEnv* env, jclass clazz ATTRIBUTE_UNUSED, jobject inf_ref) { + jclass native_inf_class = env->FindClass("NativeInterface"); + CHECK(native_inf_class != nullptr); + jmethodID mid = env->GetMethodID(native_inf_class, "callback", "()V"); + CHECK(mid != nullptr); + env->CallVoidMethod(inf_ref, mid); +} + +} // namespace art diff --git a/test/044-proxy/src/Main.java b/test/044-proxy/src/Main.java index 9580871..808a32d 100644 --- a/test/044-proxy/src/Main.java +++ b/test/044-proxy/src/Main.java @@ -28,5 +28,6 @@ public class Main { WrappedThrow.main(null); NarrowingTest.main(null); FloatSelect.main(null); + NativeProxy.main(null); } } diff --git a/test/044-proxy/src/NativeProxy.java b/test/044-proxy/src/NativeProxy.java new file mode 100644 index 0000000..954d6cc --- /dev/null +++ b/test/044-proxy/src/NativeProxy.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2015 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 java.lang.annotation.Annotation; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.Arrays; +import java.util.Comparator; + +/** + * Test invoking a proxy method from native code. + */ + +interface NativeInterface { + public void callback(); +} + +public class NativeProxy { + + public static void main(String[] args) { + System.loadLibrary("arttest"); + + try { + NativeInterface inf = (NativeInterface)Proxy.newProxyInstance( + NativeProxy.class.getClassLoader(), + new Class[] { NativeInterface.class }, + new NativeInvocationHandler()); + + nativeCall(inf); + } catch (Exception exc) { + throw new RuntimeException(exc); + } + } + + public static class NativeInvocationHandler implements InvocationHandler { + public Object invoke(final Object proxy, + final Method method, + final Object[] args) throws Throwable { + System.out.println(method.getName()); + return null; + } + } + + public static native void nativeCall(NativeInterface inf); +} diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk index 57d06c4..26eac4c 100644 --- a/test/Android.libarttest.mk +++ b/test/Android.libarttest.mk @@ -24,6 +24,7 @@ LIBARTTEST_COMMON_SRC_FILES := \ 004-ReferenceMap/stack_walk_refmap_jni.cc \ 004-StackWalk/stack_walk_jni.cc \ 004-UnsafeTest/unsafe_test.cc \ + 044-proxy/native_proxy.cc \ 051-thread/thread_test.cc \ 116-nodex2oat/nodex2oat.cc \ 117-nopatchoat/nopatchoat.cc \ -- cgit v1.1 From ddb2a98fc557753a8080f23bea79bec3b89fee2c Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 15 Oct 2015 18:19:01 -0700 Subject: Parse runtime compiler options for JIT For the case where the CppDefines do not match the device the JIT is running on. Sample logcat output to prove it works: JIT instruction set variant krait JIT instruction set features default Bug: 24982714 (cherry picked from commit 085fc87e7ea42989a4a00cacb0c9c3a6d2590af6) Change-Id: I1f4991a5d7cdc6101d1b0ecbcb39fb26dd20180a --- compiler/jit/jit_compiler.cc | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/compiler/jit/jit_compiler.cc b/compiler/jit/jit_compiler.cc index 5ef744c..13825a7 100644 --- a/compiler/jit/jit_compiler.cc +++ b/compiler/jit/jit_compiler.cc @@ -19,6 +19,7 @@ #include "art_method-inl.h" #include "arch/instruction_set.h" #include "arch/instruction_set_features.h" +#include "base/stringpiece.h" #include "base/time_utils.h" #include "base/timing_logger.h" #include "compiler_callbacks.h" @@ -86,7 +87,37 @@ JitCompiler::JitCompiler() : total_time_(0) { nullptr, false)); const InstructionSet instruction_set = kRuntimeISA; - instruction_set_features_.reset(InstructionSetFeatures::FromCppDefines()); + for (const StringPiece option : Runtime::Current()->GetCompilerOptions()) { + VLOG(compiler) << "JIT compiler option " << option; + std::string error_msg; + if (option.starts_with("--instruction-set-variant=")) { + StringPiece str = option.substr(strlen("--instruction-set-variant=")).data(); + VLOG(compiler) << "JIT instruction set variant " << str; + instruction_set_features_.reset(InstructionSetFeatures::FromVariant( + instruction_set, str.as_string(), &error_msg)); + if (instruction_set_features_ == nullptr) { + LOG(WARNING) << "Error parsing " << option << " message=" << error_msg; + } + } else if (option.starts_with("--instruction-set-features=")) { + StringPiece str = option.substr(strlen("--instruction-set-features=")).data(); + VLOG(compiler) << "JIT instruction set features " << str; + if (instruction_set_features_.get() == nullptr) { + instruction_set_features_.reset(InstructionSetFeatures::FromVariant( + instruction_set, "default", &error_msg)); + if (instruction_set_features_ == nullptr) { + LOG(WARNING) << "Error parsing " << option << " message=" << error_msg; + } + } + instruction_set_features_.reset( + instruction_set_features_->AddFeaturesFromString(str.as_string(), &error_msg)); + if (instruction_set_features_ == nullptr) { + LOG(WARNING) << "Error parsing " << option << " message=" << error_msg; + } + } + } + if (instruction_set_features_ == nullptr) { + instruction_set_features_.reset(InstructionSetFeatures::FromCppDefines()); + } cumulative_logger_.reset(new CumulativeLogger("jit times")); verification_results_.reset(new VerificationResults(compiler_options_.get())); method_inliner_map_.reset(new DexFileToMethodInlinerMap); -- cgit v1.1 From 54d8f4bc810e7e0767f44cb77e5706a232b644bb Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Fri, 16 Oct 2015 16:28:46 +0100 Subject: Fix in reference type propagation We miss updating the type of objects if their nullability gets updated first. Bug: 25008765 (cherry picked from commit 83853392e26b2aa48328bb90c9f9c57b32c280dc) Change-Id: I81aa759d96008251d74f941494abe74aa4b52bdc --- compiler/optimizing/reference_type_propagation.cc | 4 +- test/529-checker-rtp-bug/expected.txt | 0 test/529-checker-rtp-bug/info.txt | 1 + test/529-checker-rtp-bug/src/Main.java | 48 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/529-checker-rtp-bug/expected.txt create mode 100644 test/529-checker-rtp-bug/info.txt create mode 100644 test/529-checker-rtp-bug/src/Main.java diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc index 40ec46c..f8e4d10 100644 --- a/compiler/optimizing/reference_type_propagation.cc +++ b/compiler/optimizing/reference_type_propagation.cc @@ -316,7 +316,9 @@ bool ReferenceTypePropagation::UpdateNullability(HInstruction* instr) { void ReferenceTypePropagation::ProcessWorklist() { while (!worklist_.IsEmpty()) { HInstruction* instruction = worklist_.Pop(); - if (UpdateNullability(instruction) || UpdateReferenceTypeInfo(instruction)) { + bool updated_nullability = UpdateNullability(instruction); + bool updated_reference_type = UpdateReferenceTypeInfo(instruction); + if (updated_nullability || updated_reference_type) { AddDependentInstructionsToWorklist(instruction); } } diff --git a/test/529-checker-rtp-bug/expected.txt b/test/529-checker-rtp-bug/expected.txt new file mode 100644 index 0000000..e69de29 diff --git a/test/529-checker-rtp-bug/info.txt b/test/529-checker-rtp-bug/info.txt new file mode 100644 index 0000000..852cd7c --- /dev/null +++ b/test/529-checker-rtp-bug/info.txt @@ -0,0 +1 @@ +Test that we set the proper types for objects (b/25008765). diff --git a/test/529-checker-rtp-bug/src/Main.java b/test/529-checker-rtp-bug/src/Main.java new file mode 100644 index 0000000..cf5b601 --- /dev/null +++ b/test/529-checker-rtp-bug/src/Main.java @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2015 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. + */ + +final class Final { } + +public class Main { + // CHECK-START: Final Main.testKeepCheckCast(java.lang.Object, boolean) reference_type_propagation (after) + // CHECK: [[Phi:l\d+]] Phi + // CHECK: [[Class:l\d+]] LoadClass + // CHECK: CheckCast [ [[Phi]] [[Class]] ] + // CHECK: Return [ [[Phi]] ] + + // CHECK-START: Final Main.testKeepCheckCast(java.lang.Object, boolean) instruction_simplifier_after_types (after) + // CHECK: [[Phi:l\d+]] Phi + // CHECK: [[Class:l\d+]] LoadClass + // CHECK: CheckCast + // CHECK: Return [ [[Phi]] ] + public static Final testKeepCheckCast(Object o, boolean cond) { + Object x = new Final(); + while (cond) { + x = o; + cond = false; + } + return (Final) x; + } + + public static void main(String[] args) { + try { + testKeepCheckCast(new Object(), true); + throw new Error("Expected check cast exception"); + } catch (ClassCastException e) { + // expected + } + } +} -- cgit v1.1 From 7f57e8c60ec31461151a8bfdd2b3fabfa78cb3f5 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 26 Oct 2015 20:47:28 -0700 Subject: [WIP] ART: Write-protect TLS Change-Id: I6762a3a30d01bd6eb8bb25f23f390c91147fe9b4 --- build/Android.common_build.mk | 2 +- compiler/utils/arm64/assembler_arm64.cc | 2 +- runtime/arch/x86/thread_x86.cc | 2 +- runtime/asm_support.h | 15 ++- runtime/base/mutex-inl.h | 4 +- .../quick/quick_trampoline_entrypoints.cc | 1 + runtime/mem_map.cc | 31 ++++--- runtime/thread.cc | 102 +++++++++++++++++++-- runtime/thread.h | 74 ++++++++------- runtime/thread_list.cc | 2 +- 10 files changed, 171 insertions(+), 64 deletions(-) diff --git a/build/Android.common_build.mk b/build/Android.common_build.mk index b84154b..fabaaec 100644 --- a/build/Android.common_build.mk +++ b/build/Android.common_build.mk @@ -233,7 +233,7 @@ art_non_debug_cflags := \ # Cflags for debug ART and ART tools. art_debug_cflags := \ - -O2 \ + -O0 \ -DDYNAMIC_ANNOTATIONS_ENABLED=1 \ -DVIXL_DEBUG \ -UNDEBUG diff --git a/compiler/utils/arm64/assembler_arm64.cc b/compiler/utils/arm64/assembler_arm64.cc index 7d98a30..f068571 100644 --- a/compiler/utils/arm64/assembler_arm64.cc +++ b/compiler/utils/arm64/assembler_arm64.cc @@ -526,7 +526,7 @@ void Arm64Assembler::JumpTo(ManagedRegister m_base, Offset offs, ManagedRegister CHECK(scratch.IsXRegister()) << scratch; // Remove base and scratch form the temp list - higher level API uses IP1, IP0. vixl::UseScratchRegisterScope temps(vixl_masm_); - temps.Exclude(reg_x(base.AsXRegister()), reg_x(scratch.AsXRegister())); + temps.Exclude(reg_x(base.AsXRegister())); ___ Ldr(reg_x(scratch.AsXRegister()), MEM_OP(reg_x(base.AsXRegister()), offs.Int32Value())); ___ Br(reg_x(scratch.AsXRegister())); } diff --git a/runtime/arch/x86/thread_x86.cc b/runtime/arch/x86/thread_x86.cc index b97c143..56b0b79 100644 --- a/runtime/arch/x86/thread_x86.cc +++ b/runtime/arch/x86/thread_x86.cc @@ -137,7 +137,7 @@ void Thread::InitCpu() { } void Thread::CleanupCpu() { - MutexLock mu(this, *Locks::modify_ldt_lock_); + MutexLock mu(nullptr, *Locks::modify_ldt_lock_); // Sanity check that reads from %fs point to this Thread*. Thread* self_check; diff --git a/runtime/asm_support.h b/runtime/asm_support.h index 10ed0f4..69b26eb 100644 --- a/runtime/asm_support.h +++ b/runtime/asm_support.h @@ -89,7 +89,11 @@ ADD_TEST_EQ(THREAD_ID_OFFSET, art::Thread::ThinLockIdOffset<__SIZEOF_POINTER__>().Int32Value()) // Offset of field Thread::tlsPtr_.card_table. -#define THREAD_CARD_TABLE_OFFSET 128 +#if defined(__LP64__) +#define THREAD_CARD_TABLE_OFFSET 160 +#else +#define THREAD_CARD_TABLE_OFFSET 144 +#endif ADD_TEST_EQ(THREAD_CARD_TABLE_OFFSET, art::Thread::CardTableOffset<__SIZEOF_POINTER__>().Int32Value()) @@ -104,11 +108,16 @@ ADD_TEST_EQ(THREAD_TOP_QUICK_FRAME_OFFSET, art::Thread::TopOfManagedStackOffset<__SIZEOF_POINTER__>().Int32Value()) // Offset of field Thread::tlsPtr_.managed_stack.top_quick_frame_. -#define THREAD_SELF_OFFSET (THREAD_CARD_TABLE_OFFSET + (9 * __SIZEOF_POINTER__)) +#if defined(__LP64__) +#define THREAD_SELF_OFFSET 2112 +#else +#define THREAD_SELF_OFFSET 1120 +#endif +// (THREAD_CARD_TABLE_OFFSET + (9 * __SIZEOF_POINTER__)) ADD_TEST_EQ(THREAD_SELF_OFFSET, art::Thread::SelfOffset<__SIZEOF_POINTER__>().Int32Value()) -#define THREAD_LOCAL_POS_OFFSET (THREAD_CARD_TABLE_OFFSET + 147 * __SIZEOF_POINTER__) +#define THREAD_LOCAL_POS_OFFSET (THREAD_CARD_TABLE_OFFSET + 31 * __SIZEOF_POINTER__) ADD_TEST_EQ(THREAD_LOCAL_POS_OFFSET, art::Thread::ThreadLocalPosOffset<__SIZEOF_POINTER__>().Int32Value()) #define THREAD_LOCAL_END_OFFSET (THREAD_LOCAL_POS_OFFSET + __SIZEOF_POINTER__) diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h index 87840e7..2ae3b19 100644 --- a/runtime/base/mutex-inl.h +++ b/runtime/base/mutex-inl.h @@ -74,7 +74,9 @@ static inline void CheckUnattachedThread(LockLevel level) NO_THREAD_SAFETY_ANALY // Ignore logging which may or may not have set up thread data structures. level == kLoggingLock || // Avoid recursive death. - level == kAbortLock) << level; + level == kAbortLock || + // A MemMap may be created for thread objects + level == kMemMapsLock) << level; } } diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc index 2ea5cb0..73db6ba 100644 --- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc @@ -1949,6 +1949,7 @@ static TwoWordReturn artInvokeCommon(uint32_t method_idx, mirror::Object* this_o ArtMethod* caller_method, Thread* self, ArtMethod** sp) { ScopedQuickEntrypointChecks sqec(self); DCHECK_EQ(*sp, Runtime::Current()->GetCalleeSaveMethod(Runtime::kRefsAndArgs)); + DCHECK(caller_method != nullptr); ArtMethod* method = FindMethodFast(method_idx, this_object, caller_method, access_check, type); if (UNLIKELY(method == nullptr)) { const DexFile* dex_file = caller_method->GetDeclaringClass()->GetDexCache()->GetDexFile(); diff --git a/runtime/mem_map.cc b/runtime/mem_map.cc index 6566060..e13822a 100644 --- a/runtime/mem_map.cc +++ b/runtime/mem_map.cc @@ -511,26 +511,29 @@ MemMap::~MemMap() { if (base_begin_ == nullptr && base_size_ == 0) { return; } + + // Remove it from maps_. + { + MutexLock mu(Thread::Current(), *Locks::mem_maps_lock_); + bool found = false; + DCHECK(maps_ != nullptr); + for (auto it = maps_->lower_bound(base_begin_), end = maps_->end(); + it != end && it->first == base_begin_; ++it) { + if (it->second == this) { + found = true; + maps_->erase(it); + break; + } + } + CHECK(found) << "MemMap not found"; + } + if (!reuse_) { int result = munmap(base_begin_, base_size_); if (result == -1) { PLOG(FATAL) << "munmap failed"; } } - - // Remove it from maps_. - MutexLock mu(Thread::Current(), *Locks::mem_maps_lock_); - bool found = false; - DCHECK(maps_ != nullptr); - for (auto it = maps_->lower_bound(base_begin_), end = maps_->end(); - it != end && it->first == base_begin_; ++it) { - if (it->second == this) { - found = true; - maps_->erase(it); - break; - } - } - CHECK(found) << "MemMap not found"; } MemMap::MemMap(const std::string& name, uint8_t* begin, size_t size, void* base_begin, diff --git a/runtime/thread.cc b/runtime/thread.cc index 6e8f89c..a9173d5 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -96,9 +96,11 @@ void InitEntryPoints(InterpreterEntryPoints* ipoints, JniEntryPoints* jpoints, void Thread::InitTlsEntryPoints() { // Insert a placeholder so we can easily tell if we call an unimplemented entry point. - uintptr_t* begin = reinterpret_cast(&tlsPtr_.interpreter_entrypoints); - uintptr_t* end = reinterpret_cast(reinterpret_cast(&tlsPtr_.quick_entrypoints) + - sizeof(tlsPtr_.quick_entrypoints)); + uintptr_t* begin = reinterpret_cast(&tlsPtr_.quick_entrypoints); + uintptr_t* end = reinterpret_cast( + reinterpret_cast(&tlsPtr_.interpreter_entrypoints) + + sizeof(tlsPtr_.interpreter_entrypoints)); + DCHECK_LT(begin, end); for (uintptr_t* it = begin; it != end; ++it) { *it = reinterpret_cast(UnimplementedEntryPoint); } @@ -106,7 +108,90 @@ void Thread::InitTlsEntryPoints() { &tlsPtr_.quick_entrypoints); } +static constexpr bool kUseWriteProtectScheme = true; + +static size_t GetProtectionOffset() { + return RoundUp(QUICK_ENTRYPOINT_OFFSET(sizeof(void*), pInstanceofNonTrivial).Uint32Value(), 16); +} + +// Allocate a thread. This might do some magic to use two pages. +Thread* Thread::AllocateThread(bool is_daemon) { + if (!kUseWriteProtectScheme) { + return new Thread(is_daemon); + } + + std::string error_msg; + MemMap* mem_map = MemMap::MapAnonymous("some thread", + nullptr, + 2 * kPageSize, + PROT_READ | PROT_WRITE, + false, + false, + &error_msg); + if (mem_map == nullptr) { + PLOG(FATAL) << error_msg; + } + + uint8_t* second_page_address = mem_map->Begin() + kPageSize; + const uint32_t offset = GetProtectionOffset(); + uintptr_t start_address = reinterpret_cast(second_page_address) - offset; + DCHECK_GE(start_address, reinterpret_cast(mem_map->Begin()) + sizeof(void*)); + void* start_address_ptr = reinterpret_cast(start_address); + Thread* t = new (start_address_ptr) Thread(is_daemon); + + // Store a pointer to the MemMap at the bottom. + *reinterpret_cast(mem_map->Begin()) = mem_map; + + return t; +} + +static void ProtectThread(Thread* thread) { + if (!kUseWriteProtectScheme) { + return; + } + + uintptr_t thread_addr = reinterpret_cast(thread); + DCHECK_EQ(RoundUp(thread_addr, kPageSize), thread_addr + GetProtectionOffset()); + void* page_address = reinterpret_cast(RoundUp(thread_addr, kPageSize)); + mprotect(page_address, kPageSize, PROT_READ); +} + +static void UnprotectThread(Thread* thread) { + if (!kUseWriteProtectScheme) { + return; + } + + uintptr_t thread_addr = reinterpret_cast(thread); + DCHECK_EQ(RoundUp(thread_addr, kPageSize), thread_addr + GetProtectionOffset()); + void* page_address = reinterpret_cast(RoundUp(thread_addr, kPageSize)); + mprotect(page_address, kPageSize, PROT_READ | PROT_WRITE); +} + +void Thread::DeleteThread(Thread* thread) { + if (!kUseWriteProtectScheme) { + delete thread; + return; + } + + if (thread == nullptr) { + return; + } + + UnprotectThread(thread); + thread->~Thread(); + + // There should be the MemMap* at the bottom. + MemMap* mem_map = + *reinterpret_cast(RoundDown(reinterpret_cast(thread), kPageSize)); + + delete mem_map; +} + void Thread::InitStringEntryPoints() { + // Ensure things are writable. This may be a late initialization of the entrypoints for the main + // thread. + UnprotectThread(this); + ScopedObjectAccess soa(this); QuickEntryPoints* qpoints = &tlsPtr_.quick_entrypoints; qpoints->pNewEmptyString = reinterpret_cast( @@ -141,6 +226,9 @@ void Thread::InitStringEntryPoints() { soa.DecodeMethod(WellKnownClasses::java_lang_StringFactory_newStringFromStringBuffer)); qpoints->pNewStringFromStringBuilder = reinterpret_cast( soa.DecodeMethod(WellKnownClasses::java_lang_StringFactory_newStringFromStringBuilder)); + + // This is a good time to protect things, now that all entrypoints are set. + ProtectThread(this); } void Thread::ResetQuickAllocEntryPointsForThread() { @@ -406,7 +494,7 @@ void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_siz return; } - Thread* child_thread = new Thread(is_daemon); + Thread* child_thread = AllocateThread(is_daemon); // Use global JNI ref to hold peer live while child thread starts. child_thread->tlsPtr_.jpeer = env->NewGlobalRef(java_peer); stack_size = FixStackSize(stack_size); @@ -454,7 +542,7 @@ void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_siz // Manually delete the global reference since Thread::Init will not have been run. env->DeleteGlobalRef(child_thread->tlsPtr_.jpeer); child_thread->tlsPtr_.jpeer = nullptr; - delete child_thread; + DeleteThread(child_thread); child_thread = nullptr; // TODO: remove from thread group? env->SetLongField(java_peer, WellKnownClasses::java_lang_Thread_nativePeer, 0); @@ -525,11 +613,11 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_g return nullptr; } else { Runtime::Current()->StartThreadBirth(); - self = new Thread(as_daemon); + self = AllocateThread(as_daemon); bool init_success = self->Init(runtime->GetThreadList(), runtime->GetJavaVM()); Runtime::Current()->EndThreadBirth(); if (!init_success) { - delete self; + DeleteThread(self); return nullptr; } } diff --git a/runtime/thread.h b/runtime/thread.h index 0e71c08..eb1809d 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -531,7 +531,8 @@ class Thread { private: template static ThreadOffset ThreadOffsetFromTlsPtr(size_t tls_ptr_offset) { - size_t base = OFFSETOF_MEMBER(Thread, tlsPtr_); + size_t base = /* OFFSETOF_MEMBER(Thread, tlsPtr_); */ + pointer_size == 8u ? 160 : 144; size_t scale; size_t shrink; if (pointer_size == sizeof(void*)) { @@ -951,6 +952,8 @@ class Thread { ~Thread() LOCKS_EXCLUDED(Locks::mutator_lock_, Locks::thread_suspend_count_lock_); void Destroy(); + static Thread* AllocateThread(bool is_daemon); + static void DeleteThread(Thread* thread); void CreatePeer(const char* name, bool as_daemon, jobject thread_group); @@ -1132,19 +1135,31 @@ class Thread { RuntimeStats stats; } tls64_; - struct PACKED(4) tls_ptr_sized_values { - tls_ptr_sized_values() : card_table(nullptr), exception(nullptr), stack_end(nullptr), - managed_stack(), suspend_trigger(nullptr), jni_env(nullptr), tmp_jni_env(nullptr), - self(nullptr), opeer(nullptr), jpeer(nullptr), stack_begin(nullptr), stack_size(0), - stack_trace_sample(nullptr), wait_next(nullptr), monitor_enter_object(nullptr), - top_handle_scope(nullptr), class_loader_override(nullptr), long_jump_context(nullptr), - instrumentation_stack(nullptr), debug_invoke_req(nullptr), single_step_control(nullptr), - stacked_shadow_frame_record(nullptr), deoptimization_return_value_stack(nullptr), - name(nullptr), pthread_self(0), - last_no_thread_suspension_cause(nullptr), thread_local_start(nullptr), - thread_local_pos(nullptr), thread_local_end(nullptr), thread_local_objects(0), - thread_local_alloc_stack_top(nullptr), thread_local_alloc_stack_end(nullptr), - nested_signal_state(nullptr), flip_function(nullptr), method_verifier(nullptr) { + // Guards the 'interrupted_' and 'wait_monitor_' members. + Mutex* wait_mutex_ DEFAULT_MUTEX_ACQUIRED_AFTER; + + // Condition variable waited upon during a wait. + ConditionVariable* wait_cond_ GUARDED_BY(wait_mutex_); + // Pointer to the monitor lock we're currently waiting on or null if not waiting. + Monitor* wait_monitor_ GUARDED_BY(wait_mutex_); + + // Thread "interrupted" status; stays raised until queried or thrown. + bool interrupted_ GUARDED_BY(wait_mutex_); + + struct PACKED(sizeof(void*)) tls_ptr_sized_values { + tls_ptr_sized_values() : card_table(nullptr), exception(nullptr), stack_end(nullptr), + managed_stack(), suspend_trigger(nullptr), jni_env(nullptr), tmp_jni_env(nullptr), + opeer(nullptr), jpeer(nullptr), stack_begin(nullptr), stack_size(0), + stack_trace_sample(nullptr), wait_next(nullptr), monitor_enter_object(nullptr), + top_handle_scope(nullptr), class_loader_override(nullptr), long_jump_context(nullptr), + instrumentation_stack(nullptr), debug_invoke_req(nullptr), single_step_control(nullptr), + stacked_shadow_frame_record(nullptr), deoptimization_return_value_stack(nullptr), + name(nullptr), pthread_self(0), + last_no_thread_suspension_cause(nullptr), thread_local_start(nullptr), + thread_local_pos(nullptr), thread_local_end(nullptr), thread_local_objects(0), + thread_local_alloc_stack_top(nullptr), thread_local_alloc_stack_end(nullptr), + nested_signal_state(nullptr), flip_function(nullptr), method_verifier(nullptr), + self(nullptr) { std::fill(held_mutexes, held_mutexes + kLockLevelCount, nullptr); } @@ -1172,11 +1187,6 @@ class Thread { // created thread. JNIEnvExt* tmp_jni_env; - // Initialized to "this". On certain architectures (such as x86) reading off of Thread::Current - // is easy but getting the address of Thread::Current is hard. This field can be read off of - // Thread::Current to give the address. - Thread* self; - // Our managed peer (an instance of java.lang.Thread). The jobject version is used during thread // start up, until the thread is registered and the local opeer_ is used. mirror::Object* opeer; @@ -1238,12 +1248,6 @@ class Thread { // Locks::thread_suspend_count_lock_. Closure* checkpoint_functions[kMaxCheckpoints]; - // Entrypoint function pointers. - // TODO: move this to more of a global offset table model to avoid per-thread duplication. - InterpreterEntryPoints interpreter_entrypoints; - JniEntryPoints jni_entrypoints; - QuickEntryPoints quick_entrypoints; - // Thread-local allocation pointer. uint8_t* thread_local_start; uint8_t* thread_local_pos; @@ -1268,18 +1272,18 @@ class Thread { // Current method verifier, used for root marking. verifier::MethodVerifier* method_verifier; - } tlsPtr_; - - // Guards the 'interrupted_' and 'wait_monitor_' members. - Mutex* wait_mutex_ DEFAULT_MUTEX_ACQUIRED_AFTER; - // Condition variable waited upon during a wait. - ConditionVariable* wait_cond_ GUARDED_BY(wait_mutex_); - // Pointer to the monitor lock we're currently waiting on or null if not waiting. - Monitor* wait_monitor_ GUARDED_BY(wait_mutex_); + // Entrypoint function pointers. + // TODO: move this to more of a global offset table model to avoid per-thread duplication. + QuickEntryPoints quick_entrypoints; + JniEntryPoints jni_entrypoints; + InterpreterEntryPoints interpreter_entrypoints; - // Thread "interrupted" status; stays raised until queried or thrown. - bool interrupted_ GUARDED_BY(wait_mutex_); + // Initialized to "this". On certain architectures (such as x86) reading off of Thread::Current + // is easy but getting the address of Thread::Current is hard. This field can be read off of + // Thread::Current to give the address. + Thread* self; + } tlsPtr_; friend class Dbg; // For SetStateUnsafe. friend class gc::collector::SemiSpace; // For getting stack traces. diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index b697b43..7d49112 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -1148,7 +1148,7 @@ void ThreadList::Unregister(Thread* self) { } // We failed to remove the thread due to a suspend request, loop and try again. } - delete self; + Thread::DeleteThread(self); // Release the thread ID after the thread is finished and deleted to avoid cases where we can // temporarily have multiple threads with the same thread id. When this occurs, it causes -- cgit v1.1