8065634: Crash in InstanceKlass::clean_method_data when _method is NULL
Reviewed-by: coleenp, hseigel, poonam
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Thu Dec 04 09:43:12 2014 +0000
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Fri Dec 05 21:16:45 2014 +0100
@@ -3108,21 +3108,39 @@
}
}
-// Transfer ownership of metadata allocated to the InstanceKlass.
-void ClassFileParser::apply_parsed_class_metadata(
- instanceKlassHandle this_klass,
- int java_fields_count, TRAPS) {
- // Assign annotations if needed
- if (_annotations != NULL || _type_annotations != NULL ||
- _fields_annotations != NULL || _fields_type_annotations != NULL) {
+// Create the Annotations object that will
+// hold the annotations array for the Klass.
+void ClassFileParser::create_combined_annotations(TRAPS) {
+ if (_annotations == NULL &&
+ _type_annotations == NULL &&
+ _fields_annotations == NULL &&
+ _fields_type_annotations == NULL) {
+ // Don't create the Annotations object unnecessarily.
+ return;
+ }
+
Annotations* annotations = Annotations::allocate(_loader_data, CHECK);
annotations->set_class_annotations(_annotations);
annotations->set_class_type_annotations(_type_annotations);
annotations->set_fields_annotations(_fields_annotations);
annotations->set_fields_type_annotations(_fields_type_annotations);
- this_klass->set_annotations(annotations);
- }
-
+
+ // This is the Annotations object that will be
+ // assigned to InstanceKlass being constructed.
+ _combined_annotations = annotations;
+
+ // The annotations arrays below has been transfered the
+ // _combined_annotations so these fields can now be cleared.
+ _annotations = NULL;
+ _type_annotations = NULL;
+ _fields_annotations = NULL;
+ _fields_type_annotations = NULL;
+}
+
+// Transfer ownership of metadata allocated to the InstanceKlass.
+void ClassFileParser::apply_parsed_class_metadata(
+ instanceKlassHandle this_klass,
+ int java_fields_count, TRAPS) {
_cp->set_pool_holder(this_klass());
this_klass->set_constants(_cp);
this_klass->set_fields(_fields, java_fields_count);
@@ -3130,6 +3148,7 @@
this_klass->set_inner_classes(_inner_classes);
this_klass->set_local_interfaces(_local_interfaces);
this_klass->set_transitive_interfaces(_transitive_interfaces);
+ this_klass->set_annotations(_combined_annotations);
// Clear out these fields so they don't get deallocated by the destructor
clear_class_metadata();
@@ -4002,6 +4021,10 @@
ClassAnnotationCollector parsed_annotations;
parse_classfile_attributes(&parsed_annotations, CHECK_(nullHandle));
+ // Finalize the Annotations metadata object,
+ // now that all annotation arrays have been created.
+ create_combined_annotations(CHECK_(nullHandle));
+
// Make sure this is the end of class file stream
guarantee_property(cfs->at_eos(), "Extra bytes at the end of class file %s", CHECK_(nullHandle));
@@ -4302,10 +4325,27 @@
InstanceKlass::deallocate_interfaces(_loader_data, _super_klass(),
_local_interfaces, _transitive_interfaces);
- MetadataFactory::free_array<u1>(_loader_data, _annotations);
- MetadataFactory::free_array<u1>(_loader_data, _type_annotations);
- Annotations::free_contents(_loader_data, _fields_annotations);
- Annotations::free_contents(_loader_data, _fields_type_annotations);
+ if (_combined_annotations != NULL) {
+ // After all annotations arrays have been created, they are installed into the
+ // Annotations object that will be assigned to the InstanceKlass being created.
+
+ // Deallocate the Annotations object and the installed annotations arrays.
+ _combined_annotations->deallocate_contents(_loader_data);
+
+ // If the _combined_annotations pointer is non-NULL,
+ // then the other annotations fields should have been cleared.
+ assert(_annotations == NULL, "Should have been cleared");
+ assert(_type_annotations == NULL, "Should have been cleared");
+ assert(_fields_annotations == NULL, "Should have been cleared");
+ assert(_fields_type_annotations == NULL, "Should have been cleared");
+ } else {
+ // If the annotations arrays were not installed into the Annotations object,
+ // then they have to be deallocated explicitly.
+ MetadataFactory::free_array<u1>(_loader_data, _annotations);
+ MetadataFactory::free_array<u1>(_loader_data, _type_annotations);
+ Annotations::free_contents(_loader_data, _fields_annotations);
+ Annotations::free_contents(_loader_data, _fields_type_annotations);
+ }
clear_class_metadata();
--- a/hotspot/src/share/vm/classfile/classFileParser.hpp Thu Dec 04 09:43:12 2014 +0000
+++ b/hotspot/src/share/vm/classfile/classFileParser.hpp Fri Dec 05 21:16:45 2014 +0100
@@ -75,6 +75,7 @@
Array<u2>* _inner_classes;
Array<Klass*>* _local_interfaces;
Array<Klass*>* _transitive_interfaces;
+ Annotations* _combined_annotations;
AnnotationArray* _annotations;
AnnotationArray* _type_annotations;
Array<AnnotationArray*>* _fields_annotations;
@@ -86,6 +87,8 @@
void set_class_generic_signature_index(u2 x) { _generic_signature_index = x; }
void set_class_sde_buffer(char* x, int len) { _sde_buffer = x; _sde_length = len; }
+ void create_combined_annotations(TRAPS);
+
void init_parsed_class_attributes(ClassLoaderData* loader_data) {
_loader_data = loader_data;
_synthetic_flag = false;
@@ -110,6 +113,7 @@
_inner_classes = NULL;
_local_interfaces = NULL;
_transitive_interfaces = NULL;
+ _combined_annotations = NULL;
_annotations = _type_annotations = NULL;
_fields_annotations = _fields_type_annotations = NULL;
}