Bug ID: JDK-6319688 Incorrect locking in CMSPermGen::mem

Type: Bug
Component: hotspot
Sub-Component: gc
Affected Version: 5.0u6

Priority: P3
Status: Resolved
Resolution: Fixed
OS: generic
CPU: generic

Submitted: 2005-09-02
Updated: 2014-02-24
Resolved: 2006-01-10

Other
1.4.2_12Fixed

During the course of fixing 6316605 we found incorrectness in
locking in CMSPermGen::mem_allocate(). These were fixed in Mustang
under 6316605. This bug is being created to allow a backport
of that subset to 1.5.0 and 1.4.2 update trains.

See the suggested fix section of 6316605 for more details.

SUGGESTED FIX The following fix has been putback to Tu7b01: (an analogous fix is needed for MantisU12 (or whatever U??): Event: putback-to Parent workspace: /net/jano.sfbay/export/disk05/hotspot/ws/1.5/tiger_update7_baseline (jano.sfbay:/export/disk05/hotspot/ws/1.5/tiger_update7_baseline) Child workspace: /net/prt-web.sfbay/prt-workspaces/20060103123011.ysr.tiger_update/workspace (prt-web:/net/prt-web.sfbay/prt-workspaces/20060103123011.ysr.tiger_update/workspace) User: ysr Comment: --------------------------------------------------------- Job ID: 20060103123011.ysr.tiger_update Original workspace: neeraja:/net/spot/scratch/ysr/tiger_update Submitter: ysr Archived data: /net/prt-archiver.sfbay/data/archived_workspaces/1.5/tiger_update7_baseline/2006/20060103123011.ysr.tiger_update/ Webrev: http://analemma.sfbay.sun.com/net/prt-archiver.sfbay/data/archived_workspaces/1.5/tiger_update7_baseline/2006/20060103123011.ysr.tiger_update/workspace/webrevs/webrev-2006.01.03/index.html Fixed 6319688: Incorrect locking in CMSPermGen::mem_allocate() This is a backport to Tiger Update 7 (b01) of portions of the fixes in Mustang b53/b54/b37 involving portions of the following thread of related CRs: . 6316605 atg server crashed with CMS collector . 6319671 CMS should use Heap_lock for protecting heap resizing, instead of CMS token . 6325682 VM lockup with -XX:+UseConcMarkSweepGC while loading classes with custom classloader . 4828899 CMS: non-moving collection can happen while GC_locker::is_active() The main changes are these: . locking anomalies in perm gen allocation and collection corrected . locking anomalies in heap expansion corrected . for which a new CMS automaton state "Resizing" is introduced during which the appropriate locks are held and the correct coordination protocol followed to synchronize with the mutator threads on the one hand and with the (foreground) VM thread on the other. . for which the CMS collector does not attempt to resize the heap when GC locker is held, but rather proceeds with a non-moving collection. (This last was an expedient fix to mirror current Mustang code rather than apply a different Tiger-specific fix for fixing a locking anomaly.) Testing: PRT, refworkload, ATG (solaris/sparc) Reviewed by: Jon Approved by: J2SE Update Core Team (Darin Rieck) Files: update: src/share/vm/memory/concurrentMarkSweepGeneration.cpp update: src/share/vm/memory/concurrentMarkSweepGeneration.hpp update: src/share/vm/memory/permGen.cpp update: src/share/vm/memory/permGen.hpp Examined files: 3692 Contents Summary: 4 update 3688 no action (unchanged)

03-01-2006

SUGGESTED FIX ------- permGen.hpp ------- 66,68d65 < HeapWord* CMSPermGen::check_lock_and_allocate(bool lock_owned, size_t size); < void CMSPermGen::check_lock_and_collect(bool lock_owned, GCCause::Cause cause); < 69a67,68 > > HeapWord* mem_allocate_work(size_t size); ------- permGen.cpp ------- 95,101c95,97 < // This code is slightly complicated by the need for taking care < // of two cases: we may be calling here sometimes while holding the < // underlying cms space's free list lock and sometimes without. < // The solution is an efficient recursive lock for the free list, < // but here we use a naive and inefficient solution, that also, < // unfortunately, exposes the implementation details. FIX ME!!! < HeapWord* CMSPermGen::check_lock_and_allocate(bool lock_owned, size_t size) { --- > HeapWord* CMSPermGen::mem_allocate(size_t size) { > Mutex* lock = _gen->freelistLock(); > bool lock_owned = lock->owned_by_self(); 103,111c99,100 < return _gen->have_lock_and_allocate(size, false, false); < } < return _gen->allocate(size, false, false); < } < < void CMSPermGen::check_lock_and_collect(bool lock_owned, < GCCause::Cause cause) { < if (lock_owned) { < SharedHeap::heap()->collect_locked(cause); --- > MutexUnlocker mul(lock); > return mem_allocate_work(size); 113c102 < SharedHeap::heap()->collect(cause); --- > return mem_allocate_work(size); 117,120c106,107 < HeapWord* CMSPermGen::mem_allocate(size_t size) { < Mutex* lock = _gen->freelistLock(); < HeapWord* obj = NULL; < bool lock_owned = lock->owned_by_self(); --- > HeapWord* CMSPermGen::mem_allocate_work(size_t size) { > assert(!_gen->freelistLock()->owned_by_self(), "Potential deadlock"); 122c109,111 < obj = check_lock_and_allocate(lock_owned, size); --- > MutexLocker ml(Heap_lock); > > HeapWord* obj = _gen->allocate(size, false, false); 137,139c126,127 < check_lock_and_collect(lock_owned, < GCCause::_permanent_generation_full); < obj = check_lock_and_allocate(lock_owned, size); --- > SharedHeap::heap()->collect(GCCause::_permanent_generation_full); > obj = _gen->allocate(size, false, false); 147,148c135,136 < check_lock_and_collect(lock_owned, GCCause::_last_ditch_collection); < obj = check_lock_and_allocate(lock_owned, size); --- > SharedHeap::heap()->collect(GCCause::_last_ditch_collection); > obj = _gen->allocate(size, false, false);

02-01-2006

EVALUATION Should be fixed in 1.5.0 and 1.4.2 update releases.

02-09-2005

Relates :	JDK-6325682 - VM lockup with -XX:+UseConcMarkSweepGC while loading classes with custom classloader
Relates :	JDK-6316605 - atg server crashed with CMS collector
Relates :	JDK-2129693 - CMS should use Heap_lock for protecting heap resizing, instead of CMS token
Relates :	JDK-2129692 - CMS should use Heap_lock for protecting heap resizing, instead of CMS token
Relates :	JDK-6319671 - CMS should use Heap_lock for protecting heap resizing, instead of CMS token
Relates :	JDK-6263371 - SEGV in MarkFromRootsClosure::scanOopsInOop