Revert^2 "Fix race in CommitCodeInternal and cleanup"
This reverts commit 8dde74eb7bec8e989f34d86a01c26b0f5c7e6443.
We were failing to do a null check in cha.cc. Due to compiler
optimizations this would actually run without error on non-asan
builds, causing the error to pass testing.
Reason for revert: Fixed issue causing asan issue.
Test: run asan tests
Change-Id: Ic5492c69b7735555108d8b18c8e687ce510c4549
diff --git a/runtime/cha.cc b/runtime/cha.cc
index ccbe066..ce84e8c 100644
--- a/runtime/cha.cc
+++ b/runtime/cha.cc
@@ -636,38 +636,54 @@
// We do this under cha_lock_. Committing code also grabs this lock to
// make sure the code is only committed when all single-implementation
// assumptions are still true.
- MutexLock cha_mu(self, *Locks::cha_lock_);
- // Invalidate compiled methods that assume some virtual calls have only
- // single implementations.
- for (ArtMethod* invalidated : invalidated_single_impl_methods) {
- if (!invalidated->HasSingleImplementation()) {
- // It might have been invalidated already when other class linking is
- // going on.
- continue;
- }
- invalidated->SetHasSingleImplementation(false);
- if (invalidated->IsAbstract()) {
- // Clear the single implementation method.
- invalidated->SetSingleImplementation(nullptr, image_pointer_size);
- }
+ std::vector<std::pair<ArtMethod*, OatQuickMethodHeader*>> headers;
+ {
+ MutexLock cha_mu(self, *Locks::cha_lock_);
+ // Invalidate compiled methods that assume some virtual calls have only
+ // single implementations.
+ for (ArtMethod* invalidated : invalidated_single_impl_methods) {
+ if (!invalidated->HasSingleImplementation()) {
+ // It might have been invalidated already when other class linking is
+ // going on.
+ continue;
+ }
+ invalidated->SetHasSingleImplementation(false);
+ if (invalidated->IsAbstract()) {
+ // Clear the single implementation method.
+ invalidated->SetSingleImplementation(nullptr, image_pointer_size);
+ }
- if (runtime->IsAotCompiler()) {
- // No need to invalidate any compiled code as the AotCompiler doesn't
- // run any code.
- continue;
- }
+ if (runtime->IsAotCompiler()) {
+ // No need to invalidate any compiled code as the AotCompiler doesn't
+ // run any code.
+ continue;
+ }
- // Invalidate all dependents.
- for (const auto& dependent : GetDependents(invalidated)) {
- ArtMethod* method = dependent.first;;
- OatQuickMethodHeader* method_header = dependent.second;
- VLOG(class_linker) << "CHA invalidated compiled code for " << method->PrettyMethod();
- DCHECK(runtime->UseJitCompilation());
- runtime->GetJit()->GetCodeCache()->InvalidateCompiledCodeFor(
- method, method_header);
- dependent_method_headers.insert(method_header);
+ // Invalidate all dependents.
+ for (const auto& dependent : GetDependents(invalidated)) {
+ ArtMethod* method = dependent.first;;
+ OatQuickMethodHeader* method_header = dependent.second;
+ VLOG(class_linker) << "CHA invalidated compiled code for " << method->PrettyMethod();
+ DCHECK(runtime->UseJitCompilation());
+ // We need to call JitCodeCache::InvalidateCompiledCodeFor but we cannot do it here
+ // since it would run into problems with lock-ordering. We don't want to re-order the
+ // locks since that would make code-commit racy.
+ headers.push_back({method, method_header});
+ dependent_method_headers.insert(method_header);
+ }
+ RemoveAllDependenciesFor(invalidated);
}
- RemoveAllDependenciesFor(invalidated);
+ }
+ // Since we are still loading the class that invalidated the code it's fine we have this after
+ // getting rid of the dependency. Any calls would need to be with the old version (since the
+ // new one isn't loaded yet) which still works fine. We will deoptimize just after this to
+ // ensure everything gets the new state.
+ jit::Jit* jit = Runtime::Current()->GetJit();
+ if (jit != nullptr) {
+ jit::JitCodeCache* code_cache = jit->GetCodeCache();
+ for (const auto& pair : headers) {
+ code_cache->InvalidateCompiledCodeFor(pair.first, pair.second);
+ }
}
}