Skip to content

Commit 3e5094e

Browse files
author
Ivan Walulya
committed
8366865: Allocation GC Pauses Triggered after JVM has started shutdown
Reviewed-by: ayang, tschatzl
1 parent 360b6af commit 3e5094e

File tree

15 files changed

+80
-27
lines changed

15 files changed

+80
-27
lines changed

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,11 @@ HeapWord* G1CollectedHeap::attempt_allocation_slow(uint node_index, size_t word_
480480
log_warning(gc, alloc)("%s: Retried allocation %u times for %zu words",
481481
Thread::current()->name(), try_count, word_size);
482482
}
483+
484+
if (is_shutting_down()) {
485+
stall_for_vm_shutdown();
486+
return nullptr;
487+
}
483488
}
484489

485490
ShouldNotReachHere();
@@ -714,6 +719,11 @@ HeapWord* G1CollectedHeap::attempt_allocation_humongous(size_t word_size) {
714719
log_warning(gc, alloc)("%s: Retried allocation %u times for %zu words",
715720
Thread::current()->name(), try_count, word_size);
716721
}
722+
723+
if (is_shutting_down()) {
724+
stall_for_vm_shutdown();
725+
return nullptr;
726+
}
717727
}
718728

719729
ShouldNotReachHere();
@@ -1551,10 +1561,6 @@ jint G1CollectedHeap::initialize() {
15511561
return JNI_OK;
15521562
}
15531563

1554-
bool G1CollectedHeap::concurrent_mark_is_terminating() const {
1555-
return _cm_thread->should_terminate();
1556-
}
1557-
15581564
void G1CollectedHeap::stop() {
15591565
// Stop all concurrent threads. We do this to make sure these threads
15601566
// do not continue to execute and access resources (e.g. logging)
@@ -1881,7 +1887,7 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
18811887

18821888
// If VMOp skipped initiating concurrent marking cycle because
18831889
// we're terminating, then we're done.
1884-
if (op.terminating()) {
1890+
if (is_shutting_down()) {
18851891
LOG_COLLECT_CONCURRENTLY(cause, "skipped: terminating");
18861892
return false;
18871893
}

src/hotspot/share/gc/g1/g1CollectedHeap.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -914,9 +914,6 @@ class G1CollectedHeap : public CollectedHeap {
914914
// specified by the policy object.
915915
jint initialize() override;
916916

917-
// Returns whether concurrent mark threads (and the VM) are about to terminate.
918-
bool concurrent_mark_is_terminating() const;
919-
920917
void safepoint_synchronize_begin() override;
921918
void safepoint_synchronize_end() override;
922919

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1884,7 +1884,7 @@ bool G1ConcurrentMark::concurrent_cycle_abort() {
18841884
// nothing, but this situation should be extremely rare (a full gc after shutdown
18851885
// has been signalled is already rare), and this work should be negligible compared
18861886
// to actual full gc work.
1887-
if (!cm_thread()->in_progress() && !_g1h->concurrent_mark_is_terminating()) {
1887+
if (!cm_thread()->in_progress() && !_g1h->is_shutting_down()) {
18881888
return false;
18891889
}
18901890

src/hotspot/share/gc/g1/g1Policy.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,7 @@ bool G1Policy::should_retain_evac_failed_region(uint index) const {
669669
}
670670

671671
void G1Policy::record_pause_start_time() {
672+
assert(!_g1h->is_shutting_down(), "Invariant!");
672673
Ticks now = Ticks::now();
673674
_cur_pause_start_sec = now.seconds();
674675

@@ -1275,7 +1276,7 @@ void G1Policy::decide_on_concurrent_start_pause() {
12751276

12761277
// We should not be starting a concurrent start pause if the concurrent mark
12771278
// thread is terminating.
1278-
if (_g1h->concurrent_mark_is_terminating()) {
1279+
if (_g1h->is_shutting_down()) {
12791280
return;
12801281
}
12811282

src/hotspot/share/gc/g1/g1RemSet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ class G1MergeHeapRootsTask : public WorkerTask {
10241024
// There might actually have been scheduled multiple collections, but at that point we do
10251025
// not care that much about performance and just do the work multiple times if needed.
10261026
return (_g1h->collector_state()->clear_bitmap_in_progress() ||
1027-
_g1h->concurrent_mark_is_terminating()) &&
1027+
_g1h->is_shutting_down()) &&
10281028
hr->is_old();
10291029
}
10301030

src/hotspot/share/gc/g1/g1VMOperations.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ VM_G1TryInitiateConcMark::VM_G1TryInitiateConcMark(uint gc_count_before,
6464
_mark_in_progress(false),
6565
_cycle_already_in_progress(false),
6666
_whitebox_attached(false),
67-
_terminating(false),
6867
_gc_succeeded(false)
6968
{}
7069

@@ -83,19 +82,10 @@ void VM_G1TryInitiateConcMark::doit() {
8382

8483
GCCauseSetter x(g1h, _gc_cause);
8584

86-
// Record for handling by caller.
87-
_terminating = g1h->concurrent_mark_is_terminating();
88-
8985
_mark_in_progress = g1h->collector_state()->mark_in_progress();
9086
_cycle_already_in_progress = g1h->concurrent_mark()->cm_thread()->in_progress();
9187

92-
if (_terminating && GCCause::is_user_requested_gc(_gc_cause)) {
93-
// When terminating, the request to initiate a concurrent cycle will be
94-
// ignored by do_collection_pause_at_safepoint; instead it will just do
95-
// a young-only or mixed GC (depending on phase). For a user request
96-
// there's no point in even doing that much, so done. For some non-user
97-
// requests the alternative GC might still be needed.
98-
} else if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) {
88+
if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) {
9989
// Failure to force the next GC pause to be a concurrent start indicates
10090
// there is already a concurrent marking cycle in progress. Flags to indicate
10191
// that were already set, so return immediately.
@@ -119,7 +109,6 @@ VM_G1CollectForAllocation::VM_G1CollectForAllocation(size_t word_size,
119109

120110
void VM_G1CollectForAllocation::doit() {
121111
G1CollectedHeap* g1h = G1CollectedHeap::heap();
122-
123112
GCCauseSetter x(g1h, _gc_cause);
124113
// Try a partial collection of some kind.
125114
g1h->do_collection_pause_at_safepoint();
@@ -156,6 +145,14 @@ void VM_G1PauseConcurrent::doit() {
156145

157146
bool VM_G1PauseConcurrent::doit_prologue() {
158147
Heap_lock->lock();
148+
G1CollectedHeap* g1h = G1CollectedHeap::heap();
149+
if (g1h->is_shutting_down()) {
150+
Heap_lock->unlock();
151+
// JVM shutdown has started. This ensures that any further operations will be properly aborted
152+
// and will not interfere with the shutdown process.
153+
g1h->concurrent_mark()->abort_marking_threads();
154+
return false;
155+
}
159156
return true;
160157
}
161158

src/hotspot/share/gc/g1/g1VMOperations.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ class VM_G1TryInitiateConcMark : public VM_GC_Collect_Operation {
4848
bool _mark_in_progress;
4949
bool _cycle_already_in_progress;
5050
bool _whitebox_attached;
51-
bool _terminating;
5251
// The concurrent start pause may be cancelled for some reasons. Keep track of
5352
// this.
5453
bool _gc_succeeded;
@@ -63,7 +62,6 @@ class VM_G1TryInitiateConcMark : public VM_GC_Collect_Operation {
6362
bool mark_in_progress() const { return _mark_in_progress; }
6463
bool cycle_already_in_progress() const { return _cycle_already_in_progress; }
6564
bool whitebox_attached() const { return _whitebox_attached; }
66-
bool terminating() const { return _terminating; }
6765
bool gc_succeeded() const { return _gc_succeeded && VM_GC_Operation::gc_succeeded(); }
6866
};
6967

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,11 @@ HeapWord* ParallelScavengeHeap::mem_allocate_work(size_t size, bool is_tlab) {
326326
assert(is_in_or_null(op.result()), "result not in heap");
327327
return op.result();
328328
}
329+
330+
if (is_shutting_down()) {
331+
stall_for_vm_shutdown();
332+
return nullptr;
333+
}
329334
}
330335

331336
// Was the gc-overhead reached inside the safepoint? If so, this mutator

src/hotspot/share/gc/serial/serialHeap.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,11 @@ HeapWord* SerialHeap::mem_allocate_work(size_t size, bool is_tlab) {
340340
break;
341341
}
342342

343+
if (is_shutting_down()) {
344+
stall_for_vm_shutdown();
345+
return nullptr;
346+
}
347+
343348
// Give a warning if we seem to be looping forever.
344349
if ((QueuedAllocationWarningCount > 0) &&
345350
(try_count % QueuedAllocationWarningCount == 0)) {

src/hotspot/share/gc/shared/collectedHeap.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,12 @@ MetaWord* CollectedHeap::satisfy_failed_metadata_allocation(ClassLoaderData* loa
385385
if (op.gc_succeeded()) {
386386
return op.result();
387387
}
388+
389+
if (is_shutting_down()) {
390+
stall_for_vm_shutdown();
391+
return nullptr;
392+
}
393+
388394
loop_count++;
389395
if ((QueuedAllocationWarningCount > 0) &&
390396
(loop_count % QueuedAllocationWarningCount == 0)) {
@@ -603,6 +609,23 @@ void CollectedHeap::post_initialize() {
603609
initialize_serviceability();
604610
}
605611

612+
bool CollectedHeap::is_shutting_down() const {
613+
return Universe::is_shutting_down();
614+
}
615+
616+
void CollectedHeap::stall_for_vm_shutdown() {
617+
assert(is_shutting_down(), "Precondition");
618+
// Stall the thread (2 seconds) instead of an indefinite wait to avoid deadlock
619+
// if the VM shutdown triggers a GC.
620+
// The 2-seconds sleep is:
621+
// - long enough to keep daemon threads stalled, while the shutdown
622+
// sequence completes in the common case.
623+
// - short enough to avoid excessive stall time if the shutdown itself
624+
// triggers a GC.
625+
JavaThread::current()->sleep(2 * MILLIUNITS);
626+
log_warning(gc, alloc)("%s: Stall for VM-Shutdown timed out; allocation may fail with OOME", Thread::current()->name());
627+
}
628+
606629
void CollectedHeap::before_exit() {
607630
print_tracing_info();
608631

0 commit comments

Comments
 (0)