8156871: Possible concurrency issue with JVM_AddModuleExports
Lois Foltan
lois.foltan at oracle.com
Thu Jun 16 14:48:07 UTC 2016
On 6/16/2016 9:37 AM, Christian Tornqvist wrote:
> Hi Lois,
>
> Ok, I see why @compile won't work for you. We should probably look at moving the CompilerUtils code into one of the shared testlibrary classes at some point, I don't think your test is the last that will need this functionality.
>
> As for testng, you can avoid using testng by simply changing the testExportModuleStress() method to be the main method and calling compileAll() from that.
Hi Christian,
I have removed the use of testng as you suggest and moved
CompilerUtils.java up one level to the hotspot/test/runtime/modules
directory so it can be used to develop other module tests until we have
a chance to move it into the shared testlibrary classes.
http://cr.openjdk.java.net/~lfoltan/bug_jdk8156871.4/webrev/
I believe I have the required reviews for the functional piece of this,
so just the test needs to be reviewed.
Thanks!
Lois
>
> Thanks,
> Christian
>
> -----Original Message-----
> From: Lois Foltan [mailto:lois.foltan at oracle.com]
> Sent: Wednesday, June 15, 2016 11:39 AM
> To: Christian Tornqvist <christian.tornqvist at oracle.com>
> Cc: 'hotspot-runtime-dev' <hotspot-runtime-dev at openjdk.java.net>
> Subject: Re: 8156871: Possible concurrency issue with JVM_AddModuleExports
>
>
> 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
>>
>
More information about the hotspot-runtime-dev
mailing list