Add exclusion between instrumentation and GC
Instrumentation calls VisitClasses while exclusive holding the
mutator lock. This can cause deadlocks since VisitClasses needs to
decode JNI weak globals. If the system weak slow path flag is set,
then we wait holding locks while exclusive holding the mutator lock.
This causes a deadlock since the GC cannot acquire the mutator lock
to sweep system weaks.
This fixes a deadlock seen in one of the tracing tests.
Change-Id: I580152118e068a70f309dcc19df4144afec835dd
diff --git a/runtime/gc/collector_type.h b/runtime/gc/collector_type.h
index 95ba380..416510d 100644
--- a/runtime/gc/collector_type.h
+++ b/runtime/gc/collector_type.h
@@ -40,6 +40,8 @@
kCollectorTypeHeapTrim,
// A (mostly) concurrent copying collector.
kCollectorTypeCC,
+ // Instrumentation critical section fake collector.
+ kCollectorTypeInstrumentation,
// A homogeneous space compaction collector used in background transition
// when both foreground and background collector are CMS.
kCollectorTypeHomogeneousSpaceCompact,
diff --git a/runtime/gc/gc_cause.cc b/runtime/gc/gc_cause.cc
index 6be683d..84243df 100644
--- a/runtime/gc/gc_cause.cc
+++ b/runtime/gc/gc_cause.cc
@@ -33,6 +33,7 @@
case kGcCauseDisableMovingGc: return "DisableMovingGc";
case kGcCauseHomogeneousSpaceCompact: return "HomogeneousSpaceCompact";
case kGcCauseTrim: return "HeapTrim";
+ case kGcCauseInstrumentation: return "Instrumentation";
default:
LOG(FATAL) << "Unreachable";
UNREACHABLE();
diff --git a/runtime/gc/gc_cause.h b/runtime/gc/gc_cause.h
index 0536f32d..34c7766 100644
--- a/runtime/gc/gc_cause.h
+++ b/runtime/gc/gc_cause.h
@@ -39,6 +39,8 @@
kGcCauseDisableMovingGc,
// Not a real GC cause, used when we trim the heap.
kGcCauseTrim,
+ // Not a real GC cause, used to implement exclusion between GC and instrumentation.
+ kGcCauseInstrumentation,
// GC triggered for background transition when both foreground and background collector are CMS.
kGcCauseHomogeneousSpaceCompact,
};
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 7d664fa..657fcb5 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1312,6 +1312,13 @@
ATRACE_END();
}
+void Heap::StartGC(Thread* self, GcCause cause, CollectorType collector_type) {
+ MutexLock mu(self, *gc_complete_lock_);
+ // Ensure there is only one GC at a time.
+ WaitForGcToCompleteLocked(cause, self);
+ collector_type_running_ = collector_type;
+}
+
void Heap::TrimSpaces(Thread* self) {
{
// Need to do this before acquiring the locks since we don't want to get suspended while
@@ -1319,10 +1326,7 @@
ScopedThreadStateChange tsc(self, kWaitingForGcToComplete);
// Pretend we are doing a GC to prevent background compaction from deleting the space we are
// trimming.
- MutexLock mu(self, *gc_complete_lock_);
- // Ensure there is only one GC at a time.
- WaitForGcToCompleteLocked(kGcCauseTrim, self);
- collector_type_running_ = kCollectorTypeHeapTrim;
+ StartGC(self, kGcCauseTrim, kCollectorTypeHeapTrim);
}
ATRACE_BEGIN(__FUNCTION__);
const uint64_t start_ns = NanoTime();
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index d0d0be3..cc48172 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -775,6 +775,8 @@
REQUIRES(Locks::mutator_lock_);
void LogGC(GcCause gc_cause, collector::GarbageCollector* collector);
+ void StartGC(Thread* self, GcCause cause, CollectorType collector_type)
+ REQUIRES(!*gc_complete_lock_);
void FinishGC(Thread* self, collector::GcType gc_type) REQUIRES(!*gc_complete_lock_);
// Create a mem map with a preferred base address.
@@ -1325,6 +1327,7 @@
friend class collector::MarkSweep;
friend class collector::SemiSpace;
friend class ReferenceQueue;
+ friend class ScopedGCCriticalSection;
friend class VerifyReferenceCardVisitor;
friend class VerifyReferenceVisitor;
friend class VerifyObjectVisitor;
diff --git a/runtime/gc/scoped_gc_critical_section.cc b/runtime/gc/scoped_gc_critical_section.cc
new file mode 100644
index 0000000..e7786a1
--- /dev/null
+++ b/runtime/gc/scoped_gc_critical_section.cc
@@ -0,0 +1,41 @@
+/*
+ * 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 "scoped_gc_critical_section.h"
+
+#include "gc/collector_type.h"
+#include "gc/heap.h"
+#include "runtime.h"
+#include "thread-inl.h"
+
+namespace art {
+namespace gc {
+
+ScopedGCCriticalSection::ScopedGCCriticalSection(Thread* self,
+ GcCause cause,
+ CollectorType collector_type)
+ : self_(self) {
+ Runtime::Current()->GetHeap()->StartGC(self, cause, collector_type);
+ old_cause_ = self->StartAssertNoThreadSuspension("ScopedGCCriticalSection");
+}
+ScopedGCCriticalSection::~ScopedGCCriticalSection() {
+ self_->EndAssertNoThreadSuspension(old_cause_);
+ Runtime::Current()->GetHeap()->FinishGC(self_, collector::kGcTypeNone);
+}
+
+} // namespace gc
+} // namespace art
+
diff --git a/runtime/gc/scoped_gc_critical_section.h b/runtime/gc/scoped_gc_critical_section.h
new file mode 100644
index 0000000..ec93bca
--- /dev/null
+++ b/runtime/gc/scoped_gc_critical_section.h
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+#ifndef ART_RUNTIME_GC_SCOPED_GC_CRITICAL_SECTION_H_
+#define ART_RUNTIME_GC_SCOPED_GC_CRITICAL_SECTION_H_
+
+#include "base/mutex.h"
+#include "collector_type.h"
+#include "gc_cause.h"
+
+namespace art {
+
+class Thread;
+
+namespace gc {
+
+// Wait until the GC is finished and then prevent GC from starting until the destructor. Used
+// to prevent deadlocks in places where we call ClassLinker::VisitClass with all th threads
+// suspended.
+class ScopedGCCriticalSection {
+ public:
+ ScopedGCCriticalSection(Thread* self, GcCause cause, CollectorType collector_type)
+ ACQUIRE(Roles::uninterruptible_);
+ ~ScopedGCCriticalSection() RELEASE(Roles::uninterruptible_);
+
+ private:
+ Thread* const self_;
+ const char* old_cause_;
+};
+
+} // namespace gc
+} // namespace art
+
+#endif // ART_RUNTIME_GC_SCOPED_GC_CRITICAL_SECTION_H_