Re: RFR: JDK-8262430: doclint warnings in java.base module
On Thu, 25 Feb 2021 22:59:23 GMT, Jonathan Gibbons wrote: > Please review some simple doc fixes in the `java.base` module. Two were > reported by doclint; the spelling error was detected by the IDE. I think the broken link issue is very recent as it only had to reference InetSocketAddress when preping/adding support for Unix domain sockets. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2734
RFR: JDK-8262430: doclint warnings in java.base module
Please review some simple doc fixes in the `java.base` module. Two were reported by doclint; the spelling error was detected by the IDE. - Commit messages: - JDK-8262430: doclint warnings in java.base module Changes: https://git.openjdk.java.net/jdk/pull/2734/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2734&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262430 Stats: 3 lines in 2 files changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2734.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2734/head:pull/2734 PR: https://git.openjdk.java.net/jdk/pull/2734
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
On Thu, 25 Feb 2021 19:16:58 GMT, Conor Cleary wrote:
>> test/jdk/java/net/InetAddress/InternalNameServiceWithNoHostsFileTest.java
>> line 32:
>>
>>> 30: * thrown
>>> 31: * @run main/othervm -Djdk.net.hosts.file=TestHosts-II
>>> -Dsun.net.inetaddr.ttl=0
>>> 32: * InternalNameServiceWithNoHostsFileTest
>>
>> shouldn't this be:
>>
>> @run main/othervm -Djdk.net.hosts.file=${test.src}/TestHosts-II
>> -Dsun.net.inetaddr.ttl=0
>
> TestHosts-II was never in the test.src directory, from what I could tell it
> was just a dummy file (as per the test name) to verify the reaction when a
> Hosts File is not found. So, to be consistent with the other changes in this
> PR where a Host File is not present, I gave just the Filename in the path.
> Are there possible issues stemming from this? One I can think of is that a
> Hosts FIle of that name happens to be present in the user.dir though that is
> probably not very likely.
Oh - OK - this is fine then
-
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
On Thu, 25 Feb 2021 17:26:00 GMT, Daniel Fuchs wrote:
>> Conor Cleary has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - 8262195: Copyrights & InternalNSTest Host File Delete
>> - 8262195: Added descriptive comments and filename
>
> test/jdk/java/net/InetAddress/InternalNameServiceWithNoHostsFileTest.java
> line 32:
>
>> 30: * thrown
>> 31: * @run main/othervm -Djdk.net.hosts.file=TestHosts-II
>> -Dsun.net.inetaddr.ttl=0
>> 32: * InternalNameServiceWithNoHostsFileTest
>
> shouldn't this be:
>
> @run main/othervm -Djdk.net.hosts.file=${test.src}/TestHosts-II
> -Dsun.net.inetaddr.ttl=0
TestHosts-II was never in the test.src directory, from what I could tell it was
just a dummy file (as per the test name) to verify the reaction when a Hosts
File is not found. So, to be consistent with the other changes in this PR where
a Host File is not present, I gave just the Filename in the path. Are there
possible issues stemming from this? One I can think of is that a Hosts FIle of
that name happens to be present in the user.dir though that is probably not
very likely.
-
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
On Thu, 25 Feb 2021 15:38:02 GMT, Conor Cleary wrote:
>> A number of net tests use a
>> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)**
>> to verify either the functionality of this type of Name Service or as a
>> complement to other tests (such as Caching, Address Format etc.).
>>
>> Intermittent failures of these tests can be caused by tools used during JVM
>> start-up accessing/initialising classes sooner than may be expected which
>> can cause unexpected behaviour. Most commonly, this unexpected behaviour
>> takes the form of the
>> **[PlatformNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L927)**
>> being used despite a call to `System.setProperty("jdk.net.hosts.file",
>> ...)` in the test file. Due to the fact that **InetAddress** initialises
>> it's Implementation and Name Service fields in a static class initialiser
>> ([see L1132 of
>> InetAddress.java](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L1132))
>> and that the default mode is to use the **Platform Name Service**, setting
>> a system property in the main method to specify the use of
>> **HostsFileNameService** (with the jdk.net.hosts.file property)
has no affect if the class has been previously accessed during JVM start-up
and **InetAddress** will default to the **PlatformNameService** which is
unsuitable for this test. This explains the intermittent failures caused by the
use of `System.setProperty("jdk.net.hosts.file", ...)`.
>>
>> This fix improves the robustness of this test by specifying the use of the
>> **HostsFileNameService** in the `@run` tag for this test via the previously
>> mentioned system property. This gives certainty that the property will be
>> properly set in time for the actual run of this test after the JVM has
>> booted. An example of one the fixes...
>> * @run main/othervm -Djdk.net.hosts.file=TestToNumericFormatHosts
>> textToNumericFormat
>
> Conor Cleary has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8262195: Copyrights & InternalNSTest Host File Delete
> - 8262195: Added descriptive comments and filename
The latest changes look good to me. Thanks Conor.
-
Marked as reviewed by aefimov (Committer).
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
On Thu, 25 Feb 2021 15:38:02 GMT, Conor Cleary wrote:
>> A number of net tests use a
>> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)**
>> to verify either the functionality of this type of Name Service or as a
>> complement to other tests (such as Caching, Address Format etc.).
>>
>> Intermittent failures of these tests can be caused by tools used during JVM
>> start-up accessing/initialising classes sooner than may be expected which
>> can cause unexpected behaviour. Most commonly, this unexpected behaviour
>> takes the form of the
>> **[PlatformNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L927)**
>> being used despite a call to `System.setProperty("jdk.net.hosts.file",
>> ...)` in the test file. Due to the fact that **InetAddress** initialises
>> it's Implementation and Name Service fields in a static class initialiser
>> ([see L1132 of
>> InetAddress.java](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L1132))
>> and that the default mode is to use the **Platform Name Service**, setting
>> a system property in the main method to specify the use of
>> **HostsFileNameService** (with the jdk.net.hosts.file property)
has no affect if the class has been previously accessed during JVM start-up
and **InetAddress** will default to the **PlatformNameService** which is
unsuitable for this test. This explains the intermittent failures caused by the
use of `System.setProperty("jdk.net.hosts.file", ...)`.
>>
>> This fix improves the robustness of this test by specifying the use of the
>> **HostsFileNameService** in the `@run` tag for this test via the previously
>> mentioned system property. This gives certainty that the property will be
>> properly set in time for the actual run of this test after the JVM has
>> booted. An example of one the fixes...
>> * @run main/othervm -Djdk.net.hosts.file=TestToNumericFormatHosts
>> textToNumericFormat
>
> Conor Cleary has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8262195: Copyrights & InternalNSTest Host File Delete
> - 8262195: Added descriptive comments and filename
Except for that remark, changes look fine.
test/jdk/java/net/InetAddress/InternalNameServiceWithNoHostsFileTest.java line
32:
> 30: * thrown
> 31: * @run main/othervm -Djdk.net.hosts.file=TestHosts-II
> -Dsun.net.inetaddr.ttl=0
> 32: * InternalNameServiceWithNoHostsFileTest
shouldn't this be:
@run main/othervm -Djdk.net.hosts.file=${test.src}/TestHosts-II
-Dsun.net.inetaddr.ttl=0
-
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net [v2]
On Thu, 25 Feb 2021 16:59:07 GMT, Naoto Sato wrote:
>> Daniel Fuchs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed comment
>
> src/java.base/share/classes/java/net/InetSocketAddress.java line 326:
>
>> 324: /**
>> 325: * Throws {@code InvalidObjectException}, always.
>> 326: * @throws ObjectStreamException always
>
> Is this correct? Seems to contradict each other.
`InvalidObjectException` is a subclass of `ObjectStreamException`.
-
PR: https://git.openjdk.java.net/jdk/pull/2715
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net [v2]
On Thu, 25 Feb 2021 11:05:07 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find here a change that fixes "no comment" warnings generated by
>> `javadoc -Xdoclint` for `java.base/java.net`
>
> Daniel Fuchs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed comment
src/java.base/share/classes/java/net/InetSocketAddress.java line 326:
> 324: /**
> 325: * Throws {@code InvalidObjectException}, always.
> 326: * @throws ObjectStreamException always
Is this correct? Seems to contradict each other.
-
PR: https://git.openjdk.java.net/jdk/pull/2715
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
> A number of net tests use a
> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)**
> to verify either the functionality of this type of Name Service or as a
> complement to other tests (such as Caching, Address Format etc.).
>
> Intermittent failures of these tests can be caused by tools used during JVM
> start-up accessing/initialising classes sooner than may be expected which can
> cause unexpected behaviour. Most commonly, this unexpected behaviour takes
> the form of the
> **[PlatformNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L927)**
> being used despite a call to `System.setProperty("jdk.net.hosts.file", ...)`
> in the test file. Due to the fact that **InetAddress** initialises it's
> Implementation and Name Service fields in a static class initialiser ([see
> L1132 of
> InetAddress.java](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L1132))
> and that the default mode is to use the **Platform Name Service**, setting a
> system property in the main method to specify the use of
> **HostsFileNameService** (with the jdk.net.hosts.file property)
has no affect if the class has been previously accessed during JVM start-up
and **InetAddress** will default to the **PlatformNameService** which is
unsuitable for this test. This explains the intermittent failures caused by the
use of `System.setProperty("jdk.net.hosts.file", ...)`.
>
> This fix improves the robustness of this test by specifying the use of the
> **HostsFileNameService** in the `@run` tag for this test via the previously
> mentioned system property. This gives certainty that the property will be
> properly set in time for the actual run of this test after the JVM has
> booted. An example of one the fixes...
> * @run main/othervm -Djdk.net.hosts.file=TestToNumericFormatHosts
> textToNumericFormat
Conor Cleary has updated the pull request incrementally with two additional
commits since the last revision:
- 8262195: Copyrights & InternalNSTest Host File Delete
- 8262195: Added descriptive comments and filename
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/2703/files
- new: https://git.openjdk.java.net/jdk/pull/2703/files/84a0cb2c..f7e0464f
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2703&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2703&range=00-01
Stats: 16 lines in 8 files changed: 2 ins; 4 del; 10 mod
Patch: https://git.openjdk.java.net/jdk/pull/2703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2703/head:pull/2703
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net
On Wed, 24 Feb 2021 20:42:52 GMT, Daniel Fuchs wrote: > Hi, > > Please find here a change that fixes "no comment" warnings generated by > `javadoc -Xdoclint` for `java.base/java.net` The CSR draft can be reviewed here: https://bugs.openjdk.java.net/browse/JDK-8262372 - PR: https://git.openjdk.java.net/jdk/pull/2715
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 24 Feb 2021 08:50:36 GMT, Alan Bateman wrote: >> Сергей Цыпанов has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8261880: Remove static from declarations of Holder nested classes > > src/java.base/windows/classes/sun/nio/ch/PipeImpl.java line 67: > >> 65: private final SinkChannel sink; >> 66: >> 67: private static class Initializer > > This one is okay to do. Thanks for review! Could you review the rest of the code and approve this PR, if it's fine? - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net [v2]
> Hi, > > Please find here a change that fixes "no comment" warnings generated by > `javadoc -Xdoclint` for `java.base/java.net` Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision: Fixed comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/2715/files - new: https://git.openjdk.java.net/jdk/pull/2715/files/2f039c6e..ff11d016 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2715&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2715&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2715.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2715/head:pull/2715 PR: https://git.openjdk.java.net/jdk/pull/2715
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net [v2]
On Thu, 25 Feb 2021 09:24:50 GMT, Patrick Concannon wrote: >> Daniel Fuchs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed comment > > src/java.base/share/classes/java/net/HttpRetryException.java line 48: > >> 46: >> 47: /** >> 48: * the URL to be redirected to. > > Should this begin with a capital 't' like the previous comment above? Good catch. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/2715
Integrated: 8262296 Fix remaining doclint warnings in jdk.httpserver
On Wed, 24 Feb 2021 13:58:38 GMT, Chris Hegarty wrote: > Trivial clean up of remaining doclint warnings in jdk.httpserver. > > The minor spec clarifications do not amount to a normative change, so no CSR > is required (they're documented the obvious and only possible behaviour). This pull request has now been integrated. Changeset: f79c6268 Author:Chris Hegarty URL: https://git.openjdk.java.net/jdk/commit/f79c6268 Stats: 7 lines in 5 files changed: 4 ins; 0 del; 3 mod 8262296: Fix remaining doclint warnings in jdk.httpserver Reviewed-by: dfuchs, bpb - PR: https://git.openjdk.java.net/jdk/pull/2706
Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net
On Wed, 24 Feb 2021 20:42:52 GMT, Daniel Fuchs wrote: > Hi, > > Please find here a change that fixes "no comment" warnings generated by > `javadoc -Xdoclint` for `java.base/java.net` src/java.base/share/classes/java/net/HttpRetryException.java line 48: > 46: > 47: /** > 48: * the URL to be redirected to. Should this begin with a capital 't' like the previous comment above? - PR: https://git.openjdk.java.net/jdk/pull/2715
