RFR(L) 8136930: Simplify use of module-system options by custom launchers

harold seigel harold.seigel at oracle.com
Thu Aug 4 12:46:21 UTC 2016


Hi,

Please review this update for this fix.  This webrev only shows the 
changes since the last webrev.  These changes include:

 1. Fix forJDK-8162415
    <https://bugs.openjdk.java.net/browse/JDK-8162415> - the JVM now
    prints the following message when ignoring a property and
    PrintWarnings is enabled:
    warning: Ignoring system property options whose names start with
    '-Djdk.module'.  They are reserved for internal use.

 2. Fix for JDK-8162412
    <https://bugs.openjdk.java.net/browse/JDK-8162412>
 3. Fixes a problem where JVMTI was failing two JCK vm/jvmti tests

 4. Incorporates review comments from Alan, Coleen, Dan, and Lois
 5. Fixes JTReg tests that failed due to the new option syntax.

Revised webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs2/

Thanks, Harold

On 8/2/2016 9:25 AM, harold seigel wrote:
>
> Hi Lois,
>
> Thanks for the review.  Please see comments in-line.
>
> Harold
>
>
> On 8/1/2016 8:40 PM, Lois Foltan wrote:
>>
>> On 7/17/2016 7:05 PM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review these Hotspot VM only changes to process the seven 
>>> module-specific options that have been renamed to have gnu-like 
>>> names.  JDK changes for this bug will be reviewed separately.
>>>
>>> Descriptions of these options are here 
>>> <http://openjdk.java.net/jeps/293>. For these six options, 
>>> --module-path, --upgrade-module-path, --add-modules, 
>>> --limit-modules, --add-reads, and --add-exports, the JVM just sets a 
>>> system property.  For the --patch-module option, the JVM sets a 
>>> system property and then processes the option in the same way as 
>>> when it was named -Xpatch.
>>>
>>> Additionally, the JVM now checks properties specified on the command 
>>> line.  If a property matches one of the properties used by one of 
>>> the above options then the JVM ignores the property.  This forces 
>>> users to use the explicit option when wanting to do things like add 
>>> a module or a package export.
>>>
>>> The RFR contains two new tests.  Also, many existing tests were 
>>> changed to use the new option names.
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8136930
>>>
>>> Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/
>>
>> Hi Harold,
>>
>> Overall looks good.  A couple of comments:
>>
>> src/share/vm/prims/jvmtiEnv.cpp
>> - line #3428 - The if statement is incorrect.  There are internal 
>> properties, like jdk.boot.class.path.append, whose value if non-null 
>> should be returned.
> This code will be reworked in the next version of these changes 
> because of multiple issues.
>>
>> src/share/vm/runtime/arguments.cpp
>> - Arguments::append_to_addmods_property was added before the VM 
>> starting to process --add-modules.  So with this fix, it seems like 
>> it could be simply changed to:
>>
>>     bool Arguments::append_to_addmods_property(const char* module_name) {
>>       PropertyList_unique_add(&_system_properties,
>>     Arguments::get_property("jdk.module.addmods"),
>>                                                    module_name,
>>     AppendProperty, UnwriteableProperty, InternalProperty);
>>     }
>>
>> Please consider making this change since currently it contains a lot 
>> of duplicated code that is now unnecessary.
> The one difference is that append_to_addmods_property() returns a 
> status but PropertyList_unique_add() does not.  I'll look into this a 
> bit further.
>>
>> - line #3171, should the comment be "--add-modules=java.sql" instead 
>> of "--add-modules java.sql"?
> yes.
>
> The changes suggested by you, Coleen, and Dan will be in the next 
> version of this webrev.
>
> Thanks, Harold
>>
>> Thanks,
>> Lois
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>>
>>> The changes were tested with the JCK lang and VM tests, the JTreg 
>>> hotspot tests, and the RBT hotspot nightlies.
>>>
>>> Thanks, Harold
>>
>



More information about the jigsaw-dev mailing list