JDK-7184772 : G1: Incorrect assert in HeapRegionLinkedList::add_as_head()
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 7u4
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2012-07-17
  • Updated: 2013-09-18
  • Resolved: 2012-08-07
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.

To download the current JDK release, click here.
JDK 7 JDK 8 Other
7u40Fixed 8Fixed hs24Fixed
Description
The second assert in the following code from HeapRegionLinkedList::add_as_head():

  if (_head != NULL) {
    assert(length() >  0 && _tail != NULL, hrs_ext_msg(this, "invariant"));
    from_list->_tail->set_next(_head);
  } else {
    assert(length() == 0 && _head == NULL, hrs_ext_msg(this, "invariant"));
    _tail = from_list->_tail;
  }
  _head = from_list->_head;

is incorrect. The asert is checking that _head is NULL - when we already know it is NULL from negating the condition on the if-statment.

The assertion should be checking that _tail is NULL (the invariant is that both should be NULL).

Many thanks to Brandon Mitchell at Twitter for discovering and providing the fix for the issue.

Comments
EVALUATION http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/d42fe3c3001d
14-08-2012

EVALUATION http://hg.openjdk.java.net/hsx/hotspot-emb/hotspot/rev/d42fe3c3001d
06-08-2012

EVALUATION http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/d42fe3c3001d
18-07-2012

SUGGESTED FIX I noticed that there is (I assume...) a copy/pasted assertion in heapRegionSet.cpp which looks incorrect. I believe the correct code is given by this diff: --- a/src/share/vm/gc_implementation/g1/heapRegionSet.cpp +++ b/src/share/vm/gc_implementation/g1/heapRegionSet.cpp @@ -298,7 +298,7 @@ void HeapRegionLinkedList::add_as_head(HeapRegionLinkedList* from_list) { assert(length() > 0 && _tail != NULL, hrs_ext_msg(this, "invariant")); from_list->_tail->set_next(_head); } else { - assert(length() == 0 && _head == NULL, hrs_ext_msg(this, "invariant")); + assert(length() == 0 && _tail == NULL, hrs_ext_msg(this, "invariant")); _tail = from_list->_tail; } _head = from_list->_head;
17-07-2012

EVALUATION Incorrect term used in assertion...
17-07-2012