United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7018302 newly added assert related to size of constantPoolOop causes secondary assertions or crashes
JDK-7018302 : newly added assert related to size of constantPoolOop causes secondary assertions or crashes

Details
Type:
Bug
Submit Date:
2011-02-09
Status:
Closed
Updated Date:
2011-03-07
Project Name:
JDK
Resolved Date:
2011-03-07
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs21
Fixed Versions:
hs21 (b03)

Related Reports
Backport:
Relates:

Sub Tasks

Description
Nightly testing is running into secondary assertions or crashes as a reult of a newly-added
assert in 6912621 during cosntant pool creation. This needs to be investigated to root cause,
with the assertion suitably corrected, or at least temporarily weakened or removed until root
cause and fix are found.

The assertions occur with all collectors and across a range of platforms and tests, but the
following is representative of the essential failure modes:-

http://sqeweb.sfbay.sun.com/nfs/results/vm/gtee/JDK7/NIGHTLY/VM/2011-02-08/GC_Baseline-Xinc/new_unknown_failures.html

                                    

Comments
SUGGESTED FIX

diff -r c5a923563727 src/share/vm/oops/constantPoolKlass.cpp
--- a/src/share/vm/oops/constantPoolKlass.cpp   Mon Feb 07 22:19:57 2011 -0800
+++ b/src/share/vm/oops/constantPoolKlass.cpp   Wed Feb 09 14:35:07 2011 -0800
@@ -89,7diff -r c5a923563727 src/share/vm/oops/constantPoolKlass.cpp
--- a/src/share/vm/oops/constantPoolKlass.cpp   Mon Feb 07 22:19:57 2011 -0800
+++ b/src/share/vm/oops/constantPoolKlass.cpp   Wed Feb 09 15:31:21 2011 -0800
@@ -74,8 +74,8 @@
   // Note: because we may be in this "conc_unsafe" state when allocating
   // t_oop below, which may in turn cause a GC, it is imperative that our
   // size be correct, consistent and henceforth stable, at this stage.
-  assert(c->is_parsable(), "Else size() below is unreliable");
-  DEBUG_ONLY(int sz = c->size();)
+  assert(c->is_oop() && c->is_parsable(), "Else size() below is unreliable");
+  assert(size == c->size(), "size() is wrong");
 
   // initialize tag array
   // Note: cannot introduce constant pool handle before since it is not
@@ -89,7 +89,7 @@
   pool->set_tags(tags());
 
   // Check that our size was stable at its old value.
-  assert(sz == c->size(), "size() changed");
+  assert(size == pool->size(), "size() changed");
   return pool();
 }
                                     
2011-02-09
EVALUATION

Use of a naked/unhandled oop. Fix by using handle instead.
                                     
2011-02-09
SUGGESTED FIX

The following hides all use of the naked oop, using
a handle uniformly throughout the method:-

--- old/src/share/vm/oops/constantPoolKlass.cpp Wed Feb  9 17:10:12 2011
+++ new/src/share/vm/oops/constantPoolKlass.cpp Wed Feb  9 17:10:11 2011
@@ -55,32 +55,35 @@
 constantPoolOop constantPoolKlass::allocate(int length, bool is_conc_safe, TRAPS) {
   int size = constantPoolOopDesc::object_size(length);
   KlassHandle klass (THREAD, as_klassOop());
-  constantPoolOop c =
-    (constantPoolOop)CollectedHeap::permanent_obj_allocate(klass, size, CHECK_NULL);
+  assert(klass()->is_oop(), "Can't be null, else handlizing of c below won't work");
+  constantPoolHandle pool;
+  {
+    constantPoolOop c =
+      (constantPoolOop)CollectedHeap::permanent_obj_allocate(klass, size, CHECK_NULL);
+    assert(c->klass_or_null() != NULL, "Handlizing below won't work");
+    pool = constantPoolHandle(THREAD, c);
+  }
 
-  c->set_length(length);
-  c->set_tags(NULL);
-  c->set_cache(NULL);
-  c->set_operands(NULL);
-  c->set_pool_holder(NULL);
-  c->set_flags(0);
+  pool->set_length(length);
+  pool->set_tags(NULL);
+  pool->set_cache(NULL);
+  pool->set_operands(NULL);
+  pool->set_pool_holder(NULL);
+  pool->set_flags(0);
   // only set to non-zero if constant pool is merged by RedefineClasses
-  c->set_orig_length(0);
+  pool->set_orig_length(0);
   // if constant pool may change during RedefineClasses, it is created
   // unsafe for GC concurrent processing.
-  c->set_is_conc_safe(is_conc_safe);
+  pool->set_is_conc_safe(is_conc_safe);
   // all fields are initialized; needed for GC
 
   // Note: because we may be in this "conc_unsafe" state when allocating
   // t_oop below, which may in turn cause a GC, it is imperative that our
   // size be correct, consistent and henceforth stable, at this stage.
-  assert(c->is_parsable(), "Else size() below is unreliable");
-  DEBUG_ONLY(int sz = c->size();)
+  assert(pool->is_oop() && pool->is_parsable(), "Else size() below is unreliable");
+  assert(size == pool->size(), "size() is wrong");
 
   // initialize tag array
-  // Note: cannot introduce constant pool handle before since it is not
-  //       completely initialized (no class) -> would cause assertion failure
-  constantPoolHandle pool (THREAD, c);
   typeArrayOop t_oop = oopFactory::new_permanent_byteArray(length, CHECK_NULL);
   typeArrayHandle tags (THREAD, t_oop);
   for (int index = 0; index < length; index++) {
@@ -89,7 +92,7 @@
   pool->set_tags(tags());
 
   // Check that our size was stable at its old value.
-  assert(sz == c->size(), "size() changed");
+  assert(size == pool->size(), "size() changed");
   return pool();
 }
                                     
2011-02-10
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/183658a2d0b3
                                     
2011-02-11



Hardware and Software, Engineered to Work Together