8218880: G1 crashes when issuing a periodic GC while the GCLocker is held
Summary: Do not wait for the GC locker in the periodic GC thread as the GC locker being held is an indication for being busy anyway.
Reviewed-by: kbarrett, shade
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Mon Mar 04 10:19:35 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Mon Mar 04 11:49:16 2019 +0100
@@ -2113,15 +2113,21 @@
}
void G1CollectedHeap::collect(GCCause::Cause cause) {
+ try_collect(cause, true);
+}
+
+bool G1CollectedHeap::try_collect(GCCause::Cause cause, bool retry_on_gc_failure) {
assert_heap_not_locked();
- uint gc_count_before;
- uint old_marking_count_before;
- uint full_gc_count_before;
- bool retry_gc;
+ bool gc_succeeded;
+ bool should_retry_gc;
do {
- retry_gc = false;
+ should_retry_gc = false;
+
+ uint gc_count_before;
+ uint old_marking_count_before;
+ uint full_gc_count_before;
{
MutexLocker ml(Heap_lock);
@@ -2142,19 +2148,18 @@
true, /* should_initiate_conc_mark */
g1_policy()->max_pause_time_ms());
VMThread::execute(&op);
- if (!op.pause_succeeded()) {
+ gc_succeeded = op.gc_succeeded();
+ if (!gc_succeeded && retry_on_gc_failure) {
if (old_marking_count_before == _old_marking_cycles_started) {
- retry_gc = op.should_retry_gc();
+ should_retry_gc = op.should_retry_gc();
} else {
// A Full GC happened while we were trying to schedule the
- // initial-mark GC. No point in starting a new cycle given
+ // concurrent cycle. No point in starting a new cycle given
// that the whole heap was collected anyway.
}
- if (retry_gc) {
- if (GCLocker::is_active_and_needs_gc()) {
- GCLocker::stall_until_clear();
- }
+ if (should_retry_gc && GCLocker::is_active_and_needs_gc()) {
+ GCLocker::stall_until_clear();
}
}
} else {
@@ -2169,13 +2174,16 @@
false, /* should_initiate_conc_mark */
g1_policy()->max_pause_time_ms());
VMThread::execute(&op);
+ gc_succeeded = op.gc_succeeded();
} else {
// Schedule a Full GC.
VM_G1CollectFull op(gc_count_before, full_gc_count_before, cause);
VMThread::execute(&op);
+ gc_succeeded = op.gc_succeeded();
}
}
- } while (retry_gc);
+ } while (should_retry_gc);
+ return gc_succeeded;
}
bool G1CollectedHeap::is_in(const void* p) const {
@@ -2586,7 +2594,7 @@
VMThread::execute(&op);
HeapWord* result = op.result();
- bool ret_succeeded = op.prologue_succeeded() && op.pause_succeeded();
+ bool ret_succeeded = op.prologue_succeeded() && op.gc_succeeded();
assert(result == NULL || ret_succeeded,
"the result should be NULL if the VM did not succeed");
*succeeded = ret_succeeded;
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp Mon Mar 04 10:19:35 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp Mon Mar 04 11:49:16 2019 +0100
@@ -1061,6 +1061,11 @@
// "CollectedHeap" supports.
virtual void collect(GCCause::Cause cause);
+ // Perform a collection of the heap with the given cause; if the VM operation
+ // fails to execute for any reason, retry only if retry_on_gc_failure is set.
+ // Returns whether this collection actually executed.
+ bool try_collect(GCCause::Cause cause, bool retry_on_gc_failure);
+
// True iff an evacuation has failed in the most-recent collection.
bool evacuation_failed() { return _evacuation_failed; }
--- a/src/hotspot/share/gc/g1/g1VMOperations.cpp Mon Mar 04 10:19:35 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1VMOperations.cpp Mon Mar 04 11:49:16 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -36,7 +36,7 @@
void VM_G1CollectFull::doit() {
G1CollectedHeap* g1h = G1CollectedHeap::heap();
GCCauseSetter x(g1h, _gc_cause);
- g1h->do_full_collection(false /* clear_all_soft_refs */);
+ _gc_succeeded = g1h->do_full_collection(true /* explicit_gc */, false /* clear_all_soft_refs */);
}
VM_G1CollectForAllocation::VM_G1CollectForAllocation(size_t word_size,
@@ -45,7 +45,7 @@
bool should_initiate_conc_mark,
double target_pause_time_ms) :
VM_CollectForAllocation(word_size, gc_count_before, gc_cause),
- _pause_succeeded(false),
+ _gc_succeeded(false),
_should_initiate_conc_mark(should_initiate_conc_mark),
_should_retry_gc(false),
_target_pause_time_ms(target_pause_time_ms),
@@ -85,7 +85,7 @@
if (_result != NULL) {
// If we can successfully allocate before we actually do the
// pause then we will consider this pause successful.
- _pause_succeeded = true;
+ _gc_succeeded = true;
return;
}
}
@@ -130,13 +130,13 @@
}
// Try a partial collection of some kind.
- _pause_succeeded = g1h->do_collection_pause_at_safepoint(_target_pause_time_ms);
+ _gc_succeeded = g1h->do_collection_pause_at_safepoint(_target_pause_time_ms);
- if (_pause_succeeded) {
+ if (_gc_succeeded) {
if (_word_size > 0) {
// An allocation had been requested. Do it, eventually trying a stronger
// kind of GC.
- _result = g1h->satisfy_failed_allocation(_word_size, &_pause_succeeded);
+ _result = g1h->satisfy_failed_allocation(_word_size, &_gc_succeeded);
} else {
bool should_upgrade_to_full = g1h->should_upgrade_to_full_gc(_gc_cause);
@@ -145,11 +145,11 @@
// information on how much memory has been asked for. In case there are
// absolutely no regions left to allocate into, do a maximally compacting full GC.
log_info(gc, ergo)("Attempting maximally compacting collection");
- _pause_succeeded = g1h->do_full_collection(false, /* explicit gc */
+ _gc_succeeded = g1h->do_full_collection(false, /* explicit gc */
true /* clear_all_soft_refs */);
}
}
- guarantee(_pause_succeeded, "Elevated collections during the safepoint must always succeed.");
+ guarantee(_gc_succeeded, "Elevated collections during the safepoint must always succeed.");
} else {
assert(_result == NULL, "invariant");
// The only reason for the pause to not be successful is that, the GC locker is
--- a/src/hotspot/share/gc/g1/g1VMOperations.hpp Mon Mar 04 10:19:35 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1VMOperations.hpp Mon Mar 04 11:49:16 2019 +0100
@@ -35,22 +35,26 @@
// - VM_G1CollectFull
class VM_G1CollectFull : public VM_GC_Operation {
+ bool _gc_succeeded;
+
public:
VM_G1CollectFull(uint gc_count_before,
uint full_gc_count_before,
GCCause::Cause cause) :
- VM_GC_Operation(gc_count_before, cause, full_gc_count_before, true) { }
+ VM_GC_Operation(gc_count_before, cause, full_gc_count_before, true),
+ _gc_succeeded(false) { }
virtual VMOp_Type type() const { return VMOp_G1CollectFull; }
virtual void doit();
+ bool gc_succeeded() { return _gc_succeeded; }
};
class VM_G1CollectForAllocation : public VM_CollectForAllocation {
- bool _pause_succeeded;
+ bool _gc_succeeded;
- bool _should_initiate_conc_mark;
- bool _should_retry_gc;
- double _target_pause_time_ms;
- uint _old_marking_cycles_completed_before;
+ bool _should_initiate_conc_mark;
+ bool _should_retry_gc;
+ double _target_pause_time_ms;
+ uint _old_marking_cycles_completed_before;
public:
VM_G1CollectForAllocation(size_t word_size,
@@ -63,7 +67,7 @@
virtual void doit();
virtual void doit_epilogue();
bool should_retry_gc() const { return _should_retry_gc; }
- bool pause_succeeded() { return _pause_succeeded; }
+ bool gc_succeeded() { return _gc_succeeded; }
};
// Concurrent G1 stop-the-world operations such as remark and cleanup.
--- a/src/hotspot/share/gc/g1/g1YoungRemSetSamplingThread.cpp Mon Mar 04 10:19:35 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1YoungRemSetSamplingThread.cpp Mon Mar 04 11:49:16 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -89,7 +89,10 @@
if ((os::elapsedTime() - _last_periodic_gc_attempt_s) > (G1PeriodicGCInterval / 1000.0)) {
log_debug(gc, periodic)("Checking for periodic GC.");
if (should_start_periodic_gc()) {
- Universe::heap()->collect(GCCause::_g1_periodic_collection);
+ if (!G1CollectedHeap::heap()->try_collect(GCCause::_g1_periodic_collection,
+ false /* retry_on_vmop_failure */)) {
+ log_debug(gc, periodic)("GC request denied. Skipping.");
+ }
}
_last_periodic_gc_attempt_s = os::elapsedTime();
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/gc/g1/TestPeriodicCollectionJNI.java Mon Mar 04 11:49:16 2019 +0100
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package gc.g1;
+
+/* @test
+ * @bug 8218880
+ * @summary Test that issuing a periodic collection while the GC locker is
+ * held does not crash the VM.
+ * @key gc
+ * @requires vm.gc.G1
+ * @modules java.base
+ * @run main/othervm/native
+ * -Xbootclasspath/a:.
+ * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ * -XX:+UseG1GC -XX:G1PeriodicGCInterval=100
+ * -XX:+G1PeriodicGCInvokesConcurrent
+ * -Xlog:gc,gc+periodic=debug
+ * gc.g1.TestPeriodicCollectionJNI
+ * @run main/othervm/native
+ * -Xbootclasspath/a:.
+ * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ * -XX:+UseG1GC -XX:G1PeriodicGCInterval=100
+ * -XX:-G1PeriodicGCInvokesConcurrent
+ * -Xlog:gc,gc+periodic=debug
+ * gc.g1.TestPeriodicCollectionJNI
+ */
+
+public class TestPeriodicCollectionJNI {
+ static { System.loadLibrary("TestPeriodicCollectionJNI"); }
+
+ private static native boolean blockInNative(byte[] array);
+ private static native void unblock();
+
+ public static void block() {
+ if (!blockInNative(new byte[0])) {
+ throw new RuntimeException("failed to acquire lock to dummy object");
+ }
+ }
+
+ public static void main(String[] args) throws InterruptedException {
+ long timeout = 2000;
+ long startTime = System.currentTimeMillis();
+
+ // Start thread doing JNI call
+ BlockInNative blocker = new BlockInNative();
+ blocker.start();
+
+ try {
+ // Wait for periodic GC timeout to trigger
+ while (System.currentTimeMillis() < startTime + timeout) {
+ System.out.println("Sleeping to let periodic GC trigger...");
+ Thread.sleep(200);
+ }
+ } finally {
+ unblock();
+ }
+ }
+}
+
+class BlockInNative extends Thread {
+
+ public void run() {
+ TestPeriodicCollectionJNI.block();
+ }
+
+ native void unlock();
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/gc/g1/libTestPeriodicCollectionJNI.c Mon Mar 04 11:49:16 2019 +0100
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * Native support for TestPeriodicCollectionJNI test.
+ */
+
+#include "jni.h"
+
+#ifdef WINDOWS
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+static volatile int release_critical = 0;
+
+JNIEXPORT jboolean JNICALL
+Java_gc_g1_TestPeriodicCollectionJNI_blockInNative(JNIEnv* env, jobject obj, jintArray dummy) {
+ void* native_array = (*env)->GetPrimitiveArrayCritical(env, dummy, 0);
+
+ if (native_array == NULL) {
+ return JNI_FALSE;
+ }
+
+ while (!release_critical) {
+#ifdef WINDOWS
+ Sleep(1);
+#else
+ usleep(1000);
+#endif
+ }
+
+ (*env)->ReleasePrimitiveArrayCritical(env, dummy, native_array, 0);
+
+ return JNI_TRUE;
+}
+
+JNIEXPORT void JNICALL Java_gc_g1_TestPeriodicCollectionJNI_unblock(JNIEnv *env, jobject obj)
+{
+ release_critical = 1;
+}
+
+#ifdef __cplusplus
+}
+#endif