United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-6319688 : Incorrect locking in CMSPermGen::mem_allocate()

Details
Type:
Bug
Submit Date:
2005-09-02
Status:
Resolved
Updated Date:
2014-02-24
Project Name:
JDK
Resolved Date:
2006-01-10
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
5.0u6
Fixed Versions:
5.0u7 (b01)

Related Reports
Backport:
Relates:
Relates:
Relates:
Relates:
Relates:
Relates:

Sub Tasks

Description
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.

                                    

Comments
EVALUATION

Should be fixed in 1.5.0 and 1.4.2 update releases.
                                     
2005-09-02
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);
                                     
2006-01-02
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)
                                     
2006-01-03



Hardware and Software, Engineered to Work Together