8215047: Task terminators do not complete termination in consistent state
Reviewed-by: tschatzl, minqi
--- a/src/hotspot/share/gc/shared/owstTaskTerminator.cpp Tue Jan 29 08:28:24 2019 -0500
+++ b/src/hotspot/share/gc/shared/owstTaskTerminator.cpp Tue Jan 29 08:39:04 2019 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved.
*
* 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
@@ -38,15 +38,17 @@
// Single worker, done
if (_n_threads == 1) {
_offered_termination = 1;
+ assert(!peek_in_queue_set(), "Precondition");
return true;
}
_blocker->lock_without_safepoint_check();
+ _offered_termination++;
// All arrived, done
- _offered_termination++;
if (_offered_termination == _n_threads) {
_blocker->notify_all();
_blocker->unlock();
+ assert(!peek_in_queue_set(), "Precondition");
return true;
}
@@ -59,6 +61,7 @@
if (do_spin_master_work(terminator)) {
assert(_offered_termination == _n_threads, "termination condition");
+ assert(!peek_in_queue_set(), "Precondition");
return true;
} else {
_blocker->lock_without_safepoint_check();
@@ -68,12 +71,14 @@
if (_offered_termination == _n_threads) {
_blocker->unlock();
+ assert(!peek_in_queue_set(), "Precondition");
return true;
}
}
size_t tasks = tasks_in_queue_set();
if (exit_termination(tasks, terminator)) {
+ assert_lock_strong(_blocker);
_offered_termination--;
_blocker->unlock();
return false;
@@ -153,19 +158,24 @@
_total_peeks++;
#endif
size_t tasks = tasks_in_queue_set();
- if (exit_termination(tasks, terminator)) {
+ bool exit = exit_termination(tasks, terminator);
+ {
MonitorLockerEx locker(_blocker, Mutex::_no_safepoint_check_flag);
- if (tasks >= _offered_termination - 1) {
- locker.notify_all();
- } else {
- for (; tasks > 1; tasks--) {
- locker.notify();
+ // Termination condition reached
+ if (_offered_termination == _n_threads) {
+ _spin_master = NULL;
+ return true;
+ } else if (exit) {
+ if (tasks >= _offered_termination - 1) {
+ locker.notify_all();
+ } else {
+ for (; tasks > 1; tasks--) {
+ locker.notify();
+ }
}
+ _spin_master = NULL;
+ return false;
}
- _spin_master = NULL;
- return false;
- } else if (_offered_termination == _n_threads) {
- return true;
}
}
}
--- a/src/hotspot/share/gc/shared/owstTaskTerminator.hpp Tue Jan 29 08:28:24 2019 -0500
+++ b/src/hotspot/share/gc/shared/owstTaskTerminator.hpp Tue Jan 29 08:39:04 2019 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved.
*
* 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
@@ -55,6 +55,7 @@
}
virtual ~OWSTTaskTerminator() {
+ assert(_spin_master == NULL, "Should have been reset");
assert(_blocker != NULL, "Can not be NULL");
delete _blocker;
}
--- a/src/hotspot/share/gc/shared/taskqueue.cpp Tue Jan 29 08:28:24 2019 -0500
+++ b/src/hotspot/share/gc/shared/taskqueue.cpp Tue Jan 29 08:39:04 2019 -0500
@@ -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
@@ -118,6 +118,11 @@
_queue_set(queue_set),
_offered_termination(0) {}
+ParallelTaskTerminator::~ParallelTaskTerminator() {
+ assert(_offered_termination == 0 || !peek_in_queue_set(), "Precondition");
+ assert(_offered_termination == 0 || _offered_termination == _n_threads, "Terminated or aborted" );
+}
+
bool ParallelTaskTerminator::peek_in_queue_set() {
return _queue_set->peek();
}
@@ -162,6 +167,7 @@
assert(_offered_termination <= _n_threads, "Invariant");
// Are all threads offering termination?
if (_offered_termination == _n_threads) {
+ assert(!peek_in_queue_set(), "Precondition");
return true;
} else {
// Look for more work.
@@ -211,9 +217,7 @@
#endif
if (peek_in_queue_set() ||
(terminator != NULL && terminator->should_exit_termination())) {
- Atomic::dec(&_offered_termination);
- assert(_offered_termination < _n_threads, "Invariant");
- return false;
+ return complete_or_exit_termination();
}
}
}
@@ -229,6 +233,23 @@
}
#endif
+bool ParallelTaskTerminator::complete_or_exit_termination() {
+ // If termination is ever reached, terminator should stay in such state,
+ // so that all threads see the same state
+ uint current_offered = _offered_termination;
+ uint expected_value;
+ do {
+ if (current_offered == _n_threads) {
+ assert(!peek_in_queue_set(), "Precondition");
+ return true;
+ }
+ expected_value = current_offered;
+ } while ((current_offered = Atomic::cmpxchg(current_offered - 1, &_offered_termination, current_offered)) != expected_value);
+
+ assert(_offered_termination < _n_threads, "Invariant");
+ return false;
+}
+
void ParallelTaskTerminator::reset_for_reuse() {
if (_offered_termination != 0) {
assert(_offered_termination == _n_threads,
--- a/src/hotspot/share/gc/shared/taskqueue.hpp Tue Jan 29 08:28:24 2019 -0500
+++ b/src/hotspot/share/gc/shared/taskqueue.hpp Tue Jan 29 08:39:04 2019 -0500
@@ -467,11 +467,18 @@
virtual void yield();
void sleep(uint millis);
+ // Called when exiting termination is requested.
+ // When the request is made, terminator may have already terminated
+ // (e.g. all threads are arrived and offered termination). In this case,
+ // it should ignore the request and complete the termination.
+ // Return true if termination is completed. Otherwise, return false.
+ bool complete_or_exit_termination();
public:
// "n_threads" is the number of threads to be terminated. "queue_set" is a
// queue sets of work queues of other threads.
ParallelTaskTerminator(uint n_threads, TaskQueueSetSuper* queue_set);
+ virtual ~ParallelTaskTerminator();
// The current thread has no work, and is ready to terminate if everyone
// else is. If returns "true", all threads are terminated. If returns