From b851645d1aec92a73da09c0d520a43683f2a5edb Mon Sep 17 00:00:00 2001 From: johnc Date: Wed, 13 Jan 2010 15:45:47 -0800 Subject: [PATCH] 6915005: G1: Hang in PtrQueueSet::completed_buffers_list_length with gcl001 Summary: When enqueuing a completed PtrQueue buffer, cache a local pointer to the buffer and clear the field in the PtrQueue prior to unlocking the mutex referenced by the _lock field and pass the cached local value to the enqueuing routine. This will prevent the same completed buffer being enqueued multiple times, which causes the hang. Reviewed-by: ysr --- .../vm/gc_implementation/g1/ptrQueue.cpp | 36 +++++++++++++++- .../gc_implementation/g1/ptrQueue.inline.hpp | 41 ------------------- 2 files changed, 35 insertions(+), 42 deletions(-) delete mode 100644 src/share/vm/gc_implementation/g1/ptrQueue.inline.hpp diff --git a/src/share/vm/gc_implementation/g1/ptrQueue.cpp b/src/share/vm/gc_implementation/g1/ptrQueue.cpp index 50a1967bb..4a3fac2fa 100644 --- a/src/share/vm/gc_implementation/g1/ptrQueue.cpp +++ b/src/share/vm/gc_implementation/g1/ptrQueue.cpp @@ -73,7 +73,12 @@ void PtrQueue::enqueue_known_active(void* ptr) { void PtrQueue::locking_enqueue_completed_buffer(void** buf) { assert(_lock->owned_by_self(), "Required."); + + // We have to unlock _lock (which may be Shared_DirtyCardQ_lock) before + // we acquire DirtyCardQ_CBL_mon inside enqeue_complete_buffer as they + // have the same rank and we may get the "possible deadlock" message _lock->unlock(); + qset()->enqueue_complete_buffer(buf); // We must relock only because the caller will unlock, for the normal // case. @@ -140,7 +145,36 @@ void PtrQueue::handle_zero_index() { // holding the lock if there is one). if (_buf != NULL) { if (_lock) { - locking_enqueue_completed_buffer(_buf); + assert(_lock->owned_by_self(), "Required."); + + // The current PtrQ may be the shared dirty card queue and + // may be being manipulated by more than one worker thread + // during a pause. Since the enqueuing of the completed + // buffer unlocks the Shared_DirtyCardQ_lock more than one + // worker thread can 'race' on reading the shared queue attributes + // (_buf and _index) and multiple threads can call into this + // routine for the same buffer. This will cause the completed + // buffer to be added to the CBL multiple times. + + // We "claim" the current buffer by caching value of _buf in + // a local and clearing the field while holding _lock. When + // _lock is released (while enqueueing the completed buffer) + // the thread that acquires _lock will skip this code, + // preventing the subsequent the multiple enqueue, and + // install a newly allocated buffer below. + + void** buf = _buf; // local pointer to completed buffer + _buf = NULL; // clear shared _buf field + + locking_enqueue_completed_buffer(buf); // enqueue completed buffer + + // While the current thread was enqueuing the buffer another thread + // may have a allocated a new buffer and inserted it into this pointer + // queue. If that happens then we just return so that the current + // thread doesn't overwrite the buffer allocated by the other thread + // and potentially losing some dirtied cards. + + if (_buf != NULL) return; } else { if (qset()->process_or_enqueue_complete_buffer(_buf)) { // Recycle the buffer. No allocation. diff --git a/src/share/vm/gc_implementation/g1/ptrQueue.inline.hpp b/src/share/vm/gc_implementation/g1/ptrQueue.inline.hpp deleted file mode 100644 index c3ff2260f..000000000 --- a/src/share/vm/gc_implementation/g1/ptrQueue.inline.hpp +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2001-2007 Sun Microsystems, Inc. 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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, - * CA 95054 USA or visit www.sun.com if you need additional information or - * have any questions. - * - */ - -void PtrQueue::handle_zero_index() { - assert(0 == _index, "Precondition."); - // This thread records the full buffer and allocates a new one (while - // holding the lock if there is one). - void** buf = _buf; - _buf = qset()->allocate_buffer(); - _sz = qset()->buffer_size(); - _index = _sz; - assert(0 <= _index && _index <= _sz, "Invariant."); - if (buf != NULL) { - if (_lock) { - locking_enqueue_completed_buffer(buf); - } else { - qset()->enqueue_complete_buffer(buf); - } - } -} -- GitLab