JDK-2129692 : CMS should use Heap_lock for protecting heap resizing, instead of CMS token
Type:Backport
Component:hotspot
Sub-Component:gc
Priority:P2
Status:Resolved
Resolution:Fixed
Submitted:2005-09-09
Updated:2010-04-02
Resolved:2006-03-08
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
EVALUATION
This bug has been fixed in Tiger U7b01 as a result of
the fixes needed for 6319688. As a result I am marking this
subCR fixed. Please refer to 6319688 for the fix details.
03-01-2006
SUGGESTED FIX
------- concurrentMarkSweepGeneration.hpp -------
464a465,466
> // next_state(Sweeping) = {Resizing}
> // next_state(Resizing) = {Resetting}
471,478c473,481
< Resetting = 0,
< Idling = 1,
< InitialMarking = 2,
< Marking = 3,
< Precleaning = 4,
< AbortablePreclean = 5,
< FinalMarking = 6,
< Sweeping = 7
---
> Resizing = 0,
> Resetting = 1,
> Idling = 2,
> InitialMarking = 3,
> Marking = 4,
> Precleaning = 5,
> AbortablePreclean = 6,
> FinalMarking = 7,
> Sweeping = 8
603a607,609
> // Resize the generations included in the collector.
> void compute_new_size();
>
------- concurrentMarkSweepGeneration.cpp -------
716a717,718
> assert_locked_or_safepoint(Heap_lock);
>
1519a1522,1531
> // Resize the perm generation and the tenured generation
> // after obtaining the free list locks for the
> // two generations.
> void CMSCollector::compute_new_size() {
> assert_locked_or_safepoint(Heap_lock);
> FreelistLocker z(this);
> _permGen->compute_new_size();
> _cmsGen->compute_new_size();
> }
>
1896,1897c1908,1909
< assert(_collectorState == Resetting, "Collector state change "
< "to Resetting must be done under the free_list_lock");
---
> assert(_collectorState == Resizing, "Collector state change "
> "to Resizing must be done under the free_list_lock");
1899a1912,1931
> case Resizing: {
> // Sweeping has been completed...
> // At this point the background collection has completed.
> // Don't move the call to compute_new_size() down
> // into code that might be executed if the background
> // collection was preempted.
> {
> ReleaseForegroundGC x(this); // unblock FG collection
> MutexLockerEx y(Heap_lock, Mutex::_no_safepoint_check_flag);
> CMSTokenSync z(true); // not strictly needed.
> if (_collectorState == Resizing) {
> compute_new_size();
> _collectorState = Resetting;
> } else {
> assert(_collectorState == Idling, "The state should only change"
> " because the foreground collector has finished the collection");
> }
> }
> break;
> }
1901c1933
< // sweep in sweep() has been completed
---
> // CMS heap resizing has been completed
2004,2005c2036,2037
< assert(_collectorState == Resetting, "Collector state change "
< "to Resetting must be done under the free_list_lock");
---
> assert(_collectorState == Resizing, "Collector state change "
> "to Resizing must be done under the free_list_lock");
2006a2039,2044
> case Resizing: {
> // Sweeping has been completed; the actual resize in this case
> // is done separately; nothing to be done in this state.
> _collectorState = Resetting;
> break;
> }
2008c2046
< // sweep in sweep() has been completed
---
> // The heap has been resized.
2516c2554,2555
< GCMutexLocker x(ExpandHeap_lock);
---
> assert_locked_or_safepoint(Heap_lock);
>
2597c2636
< assert_locked_or_safepoint(ExpandHeap_lock);
---
> assert_locked_or_safepoint(Heap_lock);
2605c2644
< assert_locked_or_safepoint(ExpandHeap_lock);
---
> assert_locked_or_safepoint(Heap_lock);
2636c2675
< assert_locked_or_safepoint(ExpandHeap_lock);
---
> assert_locked_or_safepoint(Heap_lock);
2646c2685
< assert_locked_or_safepoint(ExpandHeap_lock);
---
> assert_locked_or_safepoint(Heap_lock);
4361c4400
< _permGen->freelistLock(), ExpandHeap_lock);
---
> _permGen->freelistLock());
4365c4404
< _collectorState = Resetting;
---
> _collectorState = Resizing;
4376c4415
< _collectorState = Resetting;
---
> _collectorState = Resizing;
4391c4430
< // from the Sweeping state to the Resetting state must be done
---
> // from the Sweeping state to the Resizing state must be done
4394,4395c4433,4434
< assert(_collectorState == Resetting, "Change of collector state to"
< " Resetting must be done under the freelistLocks (plural)");
---
> assert(_collectorState == Resizing, "Change of collector state to"
> " Resizing must be done under the freelistLocks (plural)");
4496,4499d4534
<
< // Consider changing the size of the generation after the
< // sweep so that the amount of free space is accurate.
< gen->compute_new_size();
02-01-2006
WORK AROUND
None known.
02-01-2006
EVALUATION
See Suggested Fix section for Tiger (07) diffs.