8156871: Possible concurrency issue with JVM_AddModuleExports

[Lois Foltan]

On 6/15/2016 9:32 AM, Christian Tornqvist wrote:
> Hi Lois,
>
> I briefly looked at the test code and I have some concerns. I see the CompilerUtils class, what prevents you from using @compile in jtreg?
>
> Also, it's a testng test, in Runtime we've decided to not use testng to write our tests, is there a reason for using testng here?
Hi Christian,

Thanks for reviewing the test.  CompilerUtils provides a very convenient 
way prior to the test being run, to compile a module declaration from an 
exploded build.  All that is needed is to provide the module name and 
the directory location of the top of the exploded directory that 
contains that module declaration to the CompilerUtils::compile method.  
And it allows for providing the directory where the module definition 
once compiled, should be located.  As we develop more and more extensive 
module tests, I think it would be unfortunate to have to manually go 
through and need to specify for every java file that makes up that 
module declaration:

     @compile src/jdk.test/module-info.java
     @compile src/jdk.test/test/Main.java
     @compile src/jdk.translet/module-info.java
     @compile src/jdk.translet/translet/Main.java

And these are just simple module declarations!  If there is a better way 
to do this without the CompilerUtils functionality please let me know, I 
would be happy to change it.  testng provided a way to run 
CompilerUtils::compile prior to the actual test run.  There is 
precedence for this in other areas of the hotspot/test directory.

Thanks,
Lois

>
> Thanks,
> Christian
>
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of Lois Foltan
> Sent: Friday, June 10, 2016 10:25 AM
> To: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>
> Subject: RFR: 8156871: Possible concurrency issue with JVM_AddModuleExports
>
> 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
>