RFR 8170870: https://bugs.openjdk.java.net/browse/JDK-8170870

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Thu Dec 8 10:04:17 UTC 2016

Hi Harold,

I have a couple of comments so far.


  void PackageEntry::package_exports_do(ModuleClosure* const f) {
- assert_locked_or_safepoint(Module_lock);
    assert(f != NULL, "invariant");
+ assert(Module_lock->owned_by_self(), "should have the Module_lock");

The function is used here:

static void module_export_event_callback(PackageEntry*const package) {
   assert(package != NULL,"invariant");

   if (package->is_exported()) {
     if (package->has_qual_exports_list()) {
       . . .

It seems, the module_export_event_callback() is the only context where 
the package_exports_do is called.
But that context has the assert:

It means that it can be at a safepoint with the unlocked Module_lock.
In such a case, the new assert in the package_exports_do() would fire.
(I do not see that your closed webrev updates the 

And one question:

  void PackageEntryTable::purge_all_package_exports() {
    assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
+ if (table_size() > 0) {
+ MutexLocker module_lock(Module_lock);

The purge_all_package_exports() is called at a safepoint and it holds 
the Module_lock.
This is probably Ok because the critical section does not escape the 
and also no thread is allowed to hold the Module_lock over a safepoint.
Could you confirm that my understanding is correct here?


On 12/7/16 10:43, harold seigel wrote:
> Hi,
> Please review this fix for bug JDK-8170870.  The fix synchronizes 
> access to the class loaders' PackageEntryTable and ModuleEntryTable 
> structures by requiring callers of ClassLoaderData::modules_do() and 
> ClassLoaderData::packages_do() to lock the Module_lock.
> The lock is needed because even during a safepoint one thread could be 
> removing items from the tables while another thread is reading the 
> tables.  For example, unloading a class could cause an event 
> collection framework's JVM support to read the tables during a 
> ClassUnload event while the VM is removing entries from these 
> structures for modules and packages whose class loader got unloaded.
> The fix also cleans up management of package qualified export lists.
> Open Webrev: 
> http://cr.openjdk.java.net/~hseigel/bug_8170870/webrev/index.html
> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8170870
> The fix was tested with the JCK Lang and vm tests, the JTreg hotspot, 
> java/io, java/lang, and java/util tests and the nsk cololocated and 
> the non-colocated tests.
> Thanks, Harold

More information about the hotspot-runtime-dev mailing list