8217450: Add PackageEntry::locked_lookup_only

Reviewed-by: dholmes, shade, lfoltan

src/hotspot/share/classfile/modules.cpp

@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
@@ -206,13 +206,13 @@ static void define_javabase_module(jobject module, jstring version,
       package_table->verify_javabase_packages(pkg_list);
 
       // loop through and add any new packages for java.base
-      PackageEntry* pkg;
       for (int x = 0; x < pkg_list->length(); x++) {
         // Some of java.base's packages were added early in bootstrapping, ignore duplicates.
-        if (package_table->lookup_only(pkg_list->at(x)) == NULL) {
+        PackageEntry* pkg =
-          pkg = package_table->locked_create_entry_or_null(pkg_list->at(x), ModuleEntryTable::javabase_moduleEntry());
+          package_table->locked_create_entry_or_null(pkg_list->at(x),
-          assert(pkg != NULL, "Unable to create a " JAVA_BASE_NAME " package entry");
+                                                     ModuleEntryTable::javabase_moduleEntry());
-        }
+        assert(pkg != NULL || package_table->locked_lookup_only(pkg_list->at(x)) != NULL,
+               "Unable to create a " JAVA_BASE_NAME " package entry");
         // Unable to have a GrowableArray of TempNewSymbol.  Must decrement the refcount of
         // the Symbol* that was created above for each package. The refcount was incremented
         // by SymbolTable::new_symbol and as well by the PackageEntry creation.
@@ -388,7 +388,7 @@ void Modules::define_module(jobject module, jboolean is_open, jstring version,
 
       // Check that none of the packages exist in the class loader's package table.
       for (int x = 0; x < pkg_list->length(); x++) {
-        existing_pkg = package_table->lookup_only(pkg_list->at(x));
+        existing_pkg = package_table->locked_lookup_only(pkg_list->at(x));
         if (existing_pkg != NULL) {
           // This could be because the module was already defined.  If so,
           // report that error instead of the package error.

src/hotspot/share/classfile/packageEntry.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -216,7 +216,7 @@ void PackageEntryTable::add_entry(int index, PackageEntry* new_entry) {
 PackageEntry* PackageEntryTable::locked_create_entry_or_null(Symbol* name, ModuleEntry* module) {
   assert(Module_lock->owned_by_self(), "should have the Module_lock");
   // Check if package already exists.  Return NULL if it does.
-  if (lookup_only(name) != NULL) {
+  if (locked_lookup_only(name) != NULL) {
     return NULL;
   } else {
     PackageEntry* entry = new_entry(compute_hash(name), name, module);
@@ -227,7 +227,7 @@ PackageEntry* PackageEntryTable::locked_create_entry_or_null(Symbol* name, Modul
 
 PackageEntry* PackageEntryTable::lookup(Symbol* name, ModuleEntry* module) {
   MutexLocker ml(Module_lock);
-  PackageEntry* p = lookup_only(name);
+  PackageEntry* p = locked_lookup_only(name);
   if (p != NULL) {
     return p;
   } else {
@@ -239,7 +239,13 @@ PackageEntry* PackageEntryTable::lookup(Symbol* name, ModuleEntry* module) {
 }
 
 PackageEntry* PackageEntryTable::lookup_only(Symbol* name) {
-  MutexLockerEx ml(Module_lock->owned_by_self() ? NULL : Module_lock);
+  assert(!Module_lock->owned_by_self(), "should not have the Module_lock - use locked_lookup_only");
+  MutexLockerEx ml(Module_lock);
+  return locked_lookup_only(name);
+}
+
+PackageEntry* PackageEntryTable::locked_lookup_only(Symbol* name) {
+  assert(Module_lock->owned_by_self(), "should have the Module_lock");
   int index = index_for(name);
   for (PackageEntry* p = bucket(index); p != NULL; p = p->next()) {
     if (p->name()->fast_compare(name) == 0) {

src/hotspot/share/classfile/packageEntry.hpp

@@ -250,12 +250,18 @@ public:
   // If entry already exists, return null.  Assume Module lock was taken by caller.
   PackageEntry* locked_create_entry_or_null(Symbol* name, ModuleEntry* module);
 
-  // lookup Package with loader's package entry table, if not found add
+  // Lookup Package with loader's package entry table, add it if not found.
+  // This will acquire the Module lock.
   PackageEntry* lookup(Symbol* name, ModuleEntry* module);
 
   // Only lookup Package within loader's package entry table.
+  // This will acquire the Module lock.
   PackageEntry* lookup_only(Symbol* Package);
 
+  // Only lookup Package within loader's package entry table.  Assume Module lock
+  // was taken by caller.
+  PackageEntry* locked_lookup_only(Symbol* Package);
+
   void verify_javabase_packages(GrowableArray<Symbol*> *pkg_list);
 
   // purge dead weak references out of exported list