RFR: 8156871: Possible concurrency issue with JVM_AddModuleExports
Karen Kinnear
karen.kinnear at oracle.com
Tue Jun 14 18:57:41 UTC 2016
Lois,
Thank you for the fix and the test.
Couple of questions:
1) packageEntry.hpp
- can you please modify comment “not exported” first line “the package has not been explicitly qualified to a particular module” …
- maybe you could change this to say “the package does not have qualified or unqualified exports”. Then the qualified exports comment
takes the all unnamed into account.
2) set_unqual_exported()
Do you really want assert_locked_or_safepoint(Module_lock)?
I could see reducing the export level at a safe point - e.g. if you unload a class loader and a module, then I could see reducing
the exports list or even turning it into null.
But with today’s design I was expecting that the following transitions could not happen at a safe point and only with the lock (i.e. transitions
that increase the export):
add_qexport
set_is_exported_allUnnamed (which currently acquires the lock)
set_unqual_exported() (which currently acquires the lock)
set_unqual_exported()
thanks,
Karen
> On Jun 10, 2016, at 10:25 AM, Lois Foltan <lois.foltan at oracle.com> wrote:
>
> Hello,
>
> Please review the work to finalize a concurrency issue when setting the exported state of a PackageEntry. The work completed in bug https://bugs.openjdk.java.net/browse/JDK-8152404 actually had the side effect of fixing this bug as well. Prior to JDK-8152404, each PackageEntry determined its exported state via two flags, one of which was a general _is_exported flag followed by a another more specific state of exportability flag that determined if the package was exported qualifiedly or not to a given module. Checking these two flags, as the PackageEntry::is_* methods used to do without the Module_lock, was problematic and yielded a situation where a call to add a module to a PackageEntry's qualified exported entry list failed because it was determined that the package was unqualifiedly exported when it really was not.
>
> To complete this fix, I have removed the is_unqual_exported call prior to setting a PackageEntry's exported list. The method PackageEntry::set_exported determines what the current package export state is and acts correctly. Also, the test case has been added in this webrev since it is a good stress test case for JVM module support.
>
> Passes JPRT, java/lang, java/util RBT hotspot nightly.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8156871
> Open webrev: http://cr.openjdk.java.net/~lfoltan/bug_jdk8156871/webrev/
>
> Thanks,
> Lois
More information about the hotspot-runtime-dev
mailing list