United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7184772 G1: Incorrect assert in HeapRegionLinkedList::add_as_head()
JDK-7184772 : G1: Incorrect assert in HeapRegionLinkedList::add_as_head()

Details
Type:
Bug
Submit Date:
2012-07-17
Status:
Resolved
Updated Date:
2013-04-30
Project Name:
JDK
Resolved Date:
2012-08-07
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
7u4
Fixed Versions:
hs24 (b19)

Related Reports
Backport:
Backport:
Backport:
Backport:

Sub Tasks

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
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;
                                     
2012-07-17
EVALUATION

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

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

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

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



Hardware and Software, Engineered to Work Together