8040977: G1 crashes when run with -XX:-G1DeferredRSUpdate
Summary: When G1 is run with -XX:-G1DeferredRSUpdate, the VM crashes because of wrong initialization order of member variables. The change makes the initalization explicit, not relying on initialization order any more.
Reviewed-by: brutisso, mgerdin
--- a/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.cpp Thu Jun 26 15:48:05 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.cpp Thu Jun 26 16:01:07 2014 +0200
@@ -25,11 +25,28 @@
#include "precompiled.hpp"
#include "gc_implementation/g1/g1CollectedHeap.inline.hpp"
#include "gc_implementation/g1/g1OopClosures.inline.hpp"
+#include "gc_implementation/g1/g1ParScanThreadState.hpp"
G1ParCopyHelper::G1ParCopyHelper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state) :
G1ParClosureSuper(g1, par_scan_state), _scanned_klass(NULL),
_cm(_g1->concurrent_mark()) {}
+G1ParClosureSuper::G1ParClosureSuper(G1CollectedHeap* g1) :
+ _g1(g1), _par_scan_state(NULL), _worker_id(UINT_MAX) { }
+
G1ParClosureSuper::G1ParClosureSuper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state) :
- _g1(g1), _par_scan_state(par_scan_state),
- _worker_id(par_scan_state->queue_num()) { }
+ _g1(g1), _par_scan_state(NULL),
+ _worker_id(UINT_MAX) {
+ set_par_scan_thread_state(par_scan_state);
+}
+
+void G1ParClosureSuper::set_par_scan_thread_state(G1ParScanThreadState* par_scan_state) {
+ assert(_par_scan_state == NULL, "_par_scan_state must only be set once");
+ assert(par_scan_state != NULL, "Must set par_scan_state to non-NULL.");
+
+ _par_scan_state = par_scan_state;
+ _worker_id = par_scan_state->queue_num();
+
+ assert(_worker_id < MAX2((uint)ParallelGCThreads, 1u),
+ err_msg("The given worker id %u must be less than the number of threads %u", _worker_id, MAX2((uint)ParallelGCThreads, 1u)));
+}
--- a/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.hpp Thu Jun 26 15:48:05 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.hpp Thu Jun 26 16:01:07 2014 +0200
@@ -51,8 +51,13 @@
G1ParScanThreadState* _par_scan_state;
uint _worker_id;
public:
+ // Initializes the instance, leaving _par_scan_state uninitialized. Must be done
+ // later using the set_par_scan_thread_state() method.
+ G1ParClosureSuper(G1CollectedHeap* g1);
G1ParClosureSuper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state);
bool apply_to_weak_ref_discovered_field() { return true; }
+
+ void set_par_scan_thread_state(G1ParScanThreadState* par_scan_state);
};
class G1ParPushHeapRSClosure : public G1ParClosureSuper {
@@ -68,9 +73,8 @@
class G1ParScanClosure : public G1ParClosureSuper {
public:
- G1ParScanClosure(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state, ReferenceProcessor* rp) :
- G1ParClosureSuper(g1, par_scan_state)
- {
+ G1ParScanClosure(G1CollectedHeap* g1, ReferenceProcessor* rp) :
+ G1ParClosureSuper(g1) {
assert(_ref_processor == NULL, "sanity");
_ref_processor = rp;
}
--- a/hotspot/src/share/vm/gc_implementation/g1/g1ParScanThreadState.cpp Thu Jun 26 15:48:05 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1ParScanThreadState.cpp Thu Jun 26 16:01:07 2014 +0200
@@ -30,10 +30,6 @@
#include "oops/oop.pcgc.inline.hpp"
#include "runtime/prefetch.inline.hpp"
-#ifdef _MSC_VER // the use of 'this' below gets a warning, make it go away
-#pragma warning( disable:4355 ) // 'this' : used in base member initializer list
-#endif // _MSC_VER
-
G1ParScanThreadState::G1ParScanThreadState(G1CollectedHeap* g1h, uint queue_num, ReferenceProcessor* rp)
: _g1h(g1h),
_refs(g1h->task_queue(queue_num)),
@@ -44,9 +40,10 @@
_term_attempts(0),
_surviving_alloc_buffer(g1h->desired_plab_sz(GCAllocForSurvived)),
_tenured_alloc_buffer(g1h->desired_plab_sz(GCAllocForTenured)),
- _age_table(false), _scanner(g1h, this, rp),
+ _age_table(false), _scanner(g1h, rp),
_strong_roots_time(0), _term_time(0),
_alloc_buffer_waste(0), _undo_waste(0) {
+ _scanner.set_par_scan_thread_state(this);
// we allocate G1YoungSurvRateNumRegions plus one entries, since
// we "sacrifice" entry 0 to keep track of surviving bytes for
// non-young regions (where the age is -1)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/gc/g1/TestDeferredRSUpdate.java Thu Jun 26 16:01:07 2014 +0200
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2014, 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.
+ */
+
+/*
+ * @test TestDeferredRSUpdate
+ * @bug 8040977
+ * @summary Ensure that running with -XX:-G1DeferredRSUpdate does not crash the VM
+ * @key gc
+ * @library /testlibrary
+ */
+
+import com.oracle.java.testlibrary.ProcessTools;
+import com.oracle.java.testlibrary.OutputAnalyzer;
+
+public class TestDeferredRSUpdate {
+ public static void main(String[] args) throws Exception {
+ GCTest.main(args);
+
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
+ "-Xmx10M",
+ // G1DeferredRSUpdate is a develop option, but we cannot limit execution of this test to only debug VMs.
+ "-XX:+IgnoreUnrecognizedVMOptions",
+ "-XX:-G1DeferredRSUpdate",
+ GCTest.class.getName());
+
+ OutputAnalyzer output = new OutputAnalyzer(pb.start());
+ output.shouldHaveExitValue(0);
+ }
+
+ static class GCTest {
+ private static Object[] garbage = new Object[32];
+
+ public static void main(String [] args) {
+ System.out.println("Creating garbage");
+ // Create 128MB of garbage. This should result in at least one minor GC, with
+ // some objects copied to old gen. As references from old to young are installed,
+ // the crash due to the use before initialize occurs.
+ Object prev = null;
+ Object prevPrev = null;
+ for (int i = 0; i < 1024; i++) {
+ Object[] next = new Object[32 * 1024];
+ next[0] = prev;
+ next[1] = prevPrev;
+
+ Object[] cur = (Object[]) garbage[i % garbage.length];
+ if (cur != null) {
+ cur[0] = null;
+ cur[1] = null;
+ }
+ garbage[i % garbage.length] = next;
+
+ prevPrev = prev;
+ prev = next;
+ }
+ System.out.println("Done");
+ }
+ }
+}