8148992: VM can hang on exit if root region scanning is initiated but not executed

Reviewed-by: tschatzl, pliden, jwilhelm
This commit is contained in:
Bengt Rutisson 2016-02-10 12:56:55 +01:00
parent 75826ca4d5
commit ad3fb1dbbd
4 changed files with 25 additions and 11 deletions

View file

@ -23,6 +23,7 @@
*/ */
#include "precompiled.hpp" #include "precompiled.hpp"
#include "classfile/classLoaderData.hpp"
#include "gc/g1/concurrentMarkThread.inline.hpp" #include "gc/g1/concurrentMarkThread.inline.hpp"
#include "gc/g1/g1CollectedHeap.inline.hpp" #include "gc/g1/g1CollectedHeap.inline.hpp"
#include "gc/g1/g1CollectorPolicy.hpp" #include "gc/g1/g1CollectorPolicy.hpp"
@ -123,6 +124,7 @@ void ConcurrentMarkThread::run_service() {
// wait until started is set. // wait until started is set.
sleepBeforeNextCycle(); sleepBeforeNextCycle();
if (_should_terminate) { if (_should_terminate) {
_cm->root_regions()->cancel_scan();
break; break;
} }
@ -132,6 +134,11 @@ void ConcurrentMarkThread::run_service() {
HandleMark hm; HandleMark hm;
double cycle_start = os::elapsedVTime(); double cycle_start = os::elapsedVTime();
{
GCConcPhaseTimer(_cm, "Concurrent Clearing of Claimed Marks");
ClassLoaderDataGraph::clear_claimed_marks();
}
// We have to ensure that we finish scanning the root regions // We have to ensure that we finish scanning the root regions
// before the next GC takes place. To ensure this we have to // before the next GC takes place. To ensure this we have to
// make sure that we do not join the STS until the root regions // make sure that we do not join the STS until the root regions
@ -140,7 +147,7 @@ void ConcurrentMarkThread::run_service() {
// without the root regions have been scanned which would be a // without the root regions have been scanned which would be a
// correctness issue. // correctness issue.
if (!cm()->has_aborted()) { {
GCConcPhaseTimer(_cm, "Concurrent Root Region Scanning"); GCConcPhaseTimer(_cm, "Concurrent Root Region Scanning");
_cm->scanRootRegions(); _cm->scanRootRegions();
} }

View file

@ -1290,8 +1290,7 @@ bool G1CollectedHeap::do_full_collection(bool explicit_gc,
ref_processor_cm()->verify_no_references_recorded(); ref_processor_cm()->verify_no_references_recorded();
// Abandon current iterations of concurrent marking and concurrent // Abandon current iterations of concurrent marking and concurrent
// refinement, if any are in progress. We have to do this before // refinement, if any are in progress.
// wait_until_scan_finished() below.
concurrent_mark()->abort(); concurrent_mark()->abort();
// Make sure we'll choose a new allocation region afterwards. // Make sure we'll choose a new allocation region afterwards.

View file

@ -372,6 +372,16 @@ HeapRegion* G1CMRootRegions::claim_next() {
return res; return res;
} }
void G1CMRootRegions::notify_scan_done() {
MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
_scan_in_progress = false;
RootRegionScan_lock->notify_all();
}
void G1CMRootRegions::cancel_scan() {
notify_scan_done();
}
void G1CMRootRegions::scan_finished() { void G1CMRootRegions::scan_finished() {
assert(scan_in_progress(), "pre-condition"); assert(scan_in_progress(), "pre-condition");
@ -381,11 +391,7 @@ void G1CMRootRegions::scan_finished() {
} }
_next_survivor = NULL; _next_survivor = NULL;
{ notify_scan_done();
MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
_scan_in_progress = false;
RootRegionScan_lock->notify_all();
}
} }
bool G1CMRootRegions::wait_until_scan_finished() { bool G1CMRootRegions::wait_until_scan_finished() {
@ -978,13 +984,11 @@ public:
}; };
void G1ConcurrentMark::scanRootRegions() { void G1ConcurrentMark::scanRootRegions() {
// Start of concurrent marking.
ClassLoaderDataGraph::clear_claimed_marks();
// scan_in_progress() will have been set to true only if there was // scan_in_progress() will have been set to true only if there was
// at least one root region to scan. So, if it's false, we // at least one root region to scan. So, if it's false, we
// should not attempt to do any further work. // should not attempt to do any further work.
if (root_regions()->scan_in_progress()) { if (root_regions()->scan_in_progress()) {
assert(!has_aborted(), "Aborting before root region scanning is finished not supported.");
GCTraceConcTime(Info, gc) tt("Concurrent Root Region Scan"); GCTraceConcTime(Info, gc) tt("Concurrent Root Region Scan");
_parallel_marking_threads = calc_parallel_marking_threads(); _parallel_marking_threads = calc_parallel_marking_threads();

View file

@ -229,6 +229,8 @@ private:
volatile bool _should_abort; volatile bool _should_abort;
HeapRegion* volatile _next_survivor; HeapRegion* volatile _next_survivor;
void notify_scan_done();
public: public:
G1CMRootRegions(); G1CMRootRegions();
// We actually do most of the initialization in this method. // We actually do most of the initialization in this method.
@ -248,6 +250,8 @@ public:
// all have been claimed. // all have been claimed.
HeapRegion* claim_next(); HeapRegion* claim_next();
void cancel_scan();
// Flag that we're done with root region scanning and notify anyone // Flag that we're done with root region scanning and notify anyone
// who's waiting on it. If aborted is false, assume that all regions // who's waiting on it. If aborted is false, assume that all regions
// have been claimed. // have been claimed.