RFR 8171971: Fix timing bug in JVM management of package export lists

Lois Foltan lois.foltan at oracle.com
Thu Jan 19 15:14:29 UTC 2017


On 1/18/2017 10:05 AM, harold seigel wrote:
> Hi,
>
> Please review this fix for the package export timing holes discussed 
> in JDK-8171971.  The fix reduces the number of PackageEntry fields 
> that are used to maintain a package's export state and uses the 
> Module_lock to protect all access to these fields.
>
> Also, in cases where a package transitions from having qualified 
> exports to being unqualifiedly exported, it fixes the cleanup of its 
> qualified export list by removing the _exported_pending_delete field 
> and using just is_unqual_exported() to determine when the qualified 
> exports list can be purged (at a safepoint).
>
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8171971/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8171971
>
> The fix was tested with the hotspot, java/lang, java/util, java/io, 
> JFR, and other JTReg tests, the JCK lang and VM tests, RBT tier2 - 
> tier5 tests on LinuxX64, and the colocated and non-colocated NSK tests.

Hi Harold,

Looks good! A couple of comments in src/share/vm/classfile/packageEntry.hpp:

- line #133, the method has_qual_exports_list().  A comment should be 
added ahead of this method to indicate that even if the 
_qualified_exports list is empty the package is still considered 
qualifiedly exported and a similar comment should be added to 
PackageEntry::purge_qualified_exports().  During a GC safepoint, it 
could be the scenario that all modules on a given package's qualified 
export list die, thus leaving an allocated but empty _qualified_exports 
field.  If the _qualified_exports GrowableArray was deleted in that 
scenario, then according to the new _exports_flag and a NULL value for 
_qualified_exports, the package would erroneously not be considered 
qualifiedly exported anymore. This can be reworked in the future to have 
the _exports_flag contain definitions for all possible states.  Relying 
on the state of whether _qualified_exports field is NULL or not can be 
problematic, but I think your changes do improve upon ensuring a 
PackageEntry maintains a valid state.

- line #86 - please add a blank line for readability
- line #139 & #143 please out the return expression in parentheses.

Thanks,
Lois

>
> Thanks, Harold
>



More information about the hotspot-runtime-dev mailing list