Add visiting for class loaders in StickyMarkSweep
StickyMarkSweep clear the mark stack,
Because all reachable objects must be referenced by a root or a dirty card.
But, there are some marking hole for class object.
If some object is marked and the object and it's class object is not dirty,
Then class object cannot be marking by card table.
In previous OS including mashmellow,
Class table was maintaned by class linker
and all class object was marked with kVisitRootFlagAllRoots flag.
In N OS,
Class object is not marked with kVisitRootFlagAllRoots.
So, I added new flag to mark class object and using it StickyMarkSweep.
Added regression test in 141-class-unload.
Test: test-art-host
Merged-In: I57599e6db53b260f4c5ef466b63962141b8da5c3
(cherry picked from commit a2d1b28599e38ee0180f0f7130a879eac5be9dec)
Bug: 33924225
Change-Id: I57599e6db53b260f4c5ef466b63962141b8da5c3
Signed-off-by: Hyangseok Chae <neo.chae@lge.com>
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index a34e029..9e6519b 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1848,7 +1848,7 @@
boot_class_table_.VisitRoots(buffered_visitor);
// If tracing is enabled, then mark all the class loaders to prevent unloading.
- if (tracing_enabled) {
+ if ((flags & kVisitRootFlagClassLoader) != 0 || tracing_enabled) {
for (const ClassLoaderData& data : class_loaders_) {
GcRoot<mirror::Object> root(GcRoot<mirror::Object>(self->DecodeJObject(data.weak_root)));
root.VisitRoot(visitor, RootInfo(kRootVMInternal));
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 24cbf10..4081ec7 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -627,8 +627,7 @@
void MarkSweep::MarkConcurrentRoots(VisitRootFlags flags) {
TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings());
// Visit all runtime roots and clear dirty flags.
- Runtime::Current()->VisitConcurrentRoots(
- this, static_cast<VisitRootFlags>(flags | kVisitRootFlagNonMoving));
+ Runtime::Current()->VisitConcurrentRoots(this, flags);
}
class MarkSweep::DelayReferenceReferentVisitor {
diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h
index 92d7cf6..f4c7ba6 100644
--- a/runtime/gc/collector/mark_sweep.h
+++ b/runtime/gc/collector/mark_sweep.h
@@ -98,7 +98,7 @@
REQUIRES(!mark_stack_lock_)
SHARED_REQUIRES(Locks::mutator_lock_);
- void MarkConcurrentRoots(VisitRootFlags flags)
+ virtual void MarkConcurrentRoots(VisitRootFlags flags)
REQUIRES(Locks::heap_bitmap_lock_)
REQUIRES(!mark_stack_lock_)
SHARED_REQUIRES(Locks::mutator_lock_);
diff --git a/runtime/gc/collector/sticky_mark_sweep.cc b/runtime/gc/collector/sticky_mark_sweep.cc
index bb7e854..a2dbe3f 100644
--- a/runtime/gc/collector/sticky_mark_sweep.cc
+++ b/runtime/gc/collector/sticky_mark_sweep.cc
@@ -56,6 +56,19 @@
RecursiveMarkDirtyObjects(false, accounting::CardTable::kCardDirty - 1);
}
+void StickyMarkSweep::MarkConcurrentRoots(VisitRootFlags flags) {
+ TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings());
+ // Visit all runtime roots and clear dirty flags including class loader. This is done to prevent
+ // incorrect class unloading since the GC does not card mark when storing store the class during
+ // object allocation. Doing this for each allocation would be slow.
+ // Since the card is not dirty, it means the object may not get scanned. This can cause class
+ // unloading to occur even though the class and class loader are reachable through the object's
+ // class.
+ Runtime::Current()->VisitConcurrentRoots(
+ this,
+ static_cast<VisitRootFlags>(flags | kVisitRootFlagClassLoader));
+}
+
void StickyMarkSweep::Sweep(bool swap_bitmaps ATTRIBUTE_UNUSED) {
SweepArray(GetHeap()->GetLiveStack(), false);
}
diff --git a/runtime/gc/collector/sticky_mark_sweep.h b/runtime/gc/collector/sticky_mark_sweep.h
index abaf978..93b0b5f 100644
--- a/runtime/gc/collector/sticky_mark_sweep.h
+++ b/runtime/gc/collector/sticky_mark_sweep.h
@@ -33,6 +33,12 @@
StickyMarkSweep(Heap* heap, bool is_concurrent, const std::string& name_prefix = "");
~StickyMarkSweep() {}
+ void MarkConcurrentRoots(VisitRootFlags flags)
+ OVERRIDE
+ REQUIRES(!mark_stack_lock_)
+ REQUIRES(Locks::heap_bitmap_lock_)
+ SHARED_REQUIRES(Locks::mutator_lock_);
+
protected:
// Bind the live bits to the mark bits of bitmaps for all spaces, all spaces other than the
// alloc space will be marked as immune.
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 3b72aa7..f0cbed6 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -105,9 +105,7 @@
kVisitRootFlagStartLoggingNewRoots = 0x4,
kVisitRootFlagStopLoggingNewRoots = 0x8,
kVisitRootFlagClearRootLog = 0x10,
- // Non moving means we can have optimizations where we don't visit some roots if they are
- // definitely reachable from another location. E.g. ArtMethod and ArtField roots.
- kVisitRootFlagNonMoving = 0x20,
+ kVisitRootFlagClassLoader = 0x20,
};
class Runtime {
diff --git a/test/141-class-unload/expected.txt b/test/141-class-unload/expected.txt
index 11de660..ae5f511 100644
--- a/test/141-class-unload/expected.txt
+++ b/test/141-class-unload/expected.txt
@@ -22,3 +22,4 @@
class null false test
JNI_OnUnload called
Number of loaded unload-ex maps 0
+Too small false
diff --git a/test/141-class-unload/src/Main.java b/test/141-class-unload/src/Main.java
index 17a6049..3a05302 100644
--- a/test/141-class-unload/src/Main.java
+++ b/test/141-class-unload/src/Main.java
@@ -49,6 +49,8 @@
stressTest(constructor);
// Test that the oat files are unloaded.
testOatFilesUnloaded(getPid());
+ // Test that objects keep class loader live for sticky GC.
+ testStickyUnload(constructor);
} catch (Exception e) {
e.printStackTrace();
}
@@ -152,6 +154,30 @@
return new WeakReference(intHolder);
}
+ private static Object allocObjectInOtherClassLoader(Constructor<?> constructor)
+ throws Exception {
+ ClassLoader loader = (ClassLoader) constructor.newInstance(
+ DEX_FILE, LIBRARY_SEARCH_PATH, ClassLoader.getSystemClassLoader());
+ return loader.loadClass("IntHolder").newInstance();
+ }
+
+ // Regression test for public issue 227182.
+ private static void testStickyUnload(Constructor<?> constructor) throws Exception {
+ String s = "";
+ for (int i = 0; i < 10; ++i) {
+ s = "";
+ // The object is the only thing preventing the class loader from being unloaded.
+ Object o = allocObjectInOtherClassLoader(constructor);
+ for (int j = 0; j < 1000; ++j) {
+ s += j + " ";
+ }
+ // Make sure the object still has a valid class (hasn't been incorrectly unloaded).
+ s += o.getClass().getName();
+ o = null;
+ }
+ System.out.println("Too small " + (s.length() < 1000));
+ }
+
private static WeakReference<ClassLoader> setUpUnloadLoader(Constructor constructor,
boolean waitForCompilation)
throws Exception {
