Re: RFR: 8255674: SSLEngine class description is missing "case" in switch statement
On Tue, 25 May 2021 04:36:53 GMT, Jaikiran Pai wrote: > Can I please get a review for this trivial fix in the code sample in javadoc > comment of `javax.net.ssl.SSLEngine` class? > > I've run `make docs-image` locally and the generated javadoc after this > change looks fine. Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4180
RFR: 8255674: SSLEngine class description is missing "case" in switch statement
Can I please get a review for this trivial fix in the code sample in javadoc comment of `javax.net.ssl.SSLEngine` class? I've run `make docs-image` locally and the generated javadoc after this change looks fine. - Commit messages: - 8255674 SSLEngine class description is missing "case" in switch statement Changes: https://git.openjdk.java.net/jdk/pull/4180/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4180&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255674 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4180.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4180/head:pull/4180 PR: https://git.openjdk.java.net/jdk/pull/4180
Integrated: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64)
On Tue, 18 May 2021 22:43:14 GMT, Mark Sheppard wrote: > The test java/net/Socket/UdpSocket.java has been seen to fail with a > BindException, in the testMaxSockets test, on a regular basis on > macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP > Sockets that may be created as defined by a system property > sun.net.maxDatagramSockets. It invokes the Socket constructor > Socket(InetAddress host, int port, boolean stream) with stream set to false > to create a UDP Socket. This instantiation is a compound operation, > consisting of the creation of a socket, the explicit binding of wildcard > address and ephemeral port, and a connect to the socket end point specified > in the constructor parameters. Analysis has shown that during the test that > the OS intermittently allocates the same ephemeral port multiple times during > the bind system call, such that two separate sockets end up bound to the same > port. Then on the connect invocation a BindException is thrown for the second > socket. This has been determined to be a transient condition duri ng heavy loads and where a significant number of ephemeral ports are being allocated. > > As this is deemed an OS issues, and in order to increase test stability, it > has been found that for the BindException condition a retry of the Socket > creation mitigates the issues and tests the max creation property. This pull request has now been integrated. Changeset: bb085f68 Author:Mark Sheppard URL: https://git.openjdk.java.net/jdk/commit/bb085f684d1154ffd6b2169259c67cfb19958380 Stats: 14 lines in 2 files changed: 10 ins; 3 del; 1 mod 8265362: java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) Reviewed-by: dfuchs, alanb - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]
On Mon, 24 May 2021 09:00:07 GMT, Daniel Fuchs wrote: > Thanks for taking in my suggestions for FtpClient. I have also reviewed the > changes to java.util.logging and JMX. I wish there was a way to handle > doPrivileged returning void more nicely. Maybe component maintainers will be > able to deal with them on a case-by-case basis later on. Assigning to a useless "dummy" variable looks ugly. Extracting the call to a method might make it faraway from its caller. In L114 of `FtpClient.java` I managed to invent a return value and I thought it was perfect. But you said it's "a bit strange". :-( - PR: https://git.openjdk.java.net/jdk/pull/4138
Integrated: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote:
> Please review the test changes for [JEP
> 411](https://openjdk.java.net/jeps/411).
>
> With JEP 411 and the default value of `-Djava.security.manager` becoming
> `disallow`, tests calling `System.setSecurityManager()` need
> `-Djava.security.manager=allow` when launched. This PR covers such changes
> for tier1 to tier3 (except for the JCK tests).
>
> To make it easier to focus your review on the tests in your area, this PR is
> divided into multiple commits for different areas (~~serviceability~~,
> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d,
> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~,
> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). Mostly the rule is the
> same as how Skara adds labels, but there are several small tweaks:
>
> 1. When a file is covered by multiple labels, only one is chosen. I make my
> best to avoid putting too many tests into `core-libs`. If a file is covered
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the
> `rmi` commit.
> 4. One file not covered by any label --
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in
> the `swing` commit.
>
> Due to the size of this PR, no attempt is made to update copyright years for
> all files to minimize unnecessary merge conflict.
>
> Please note that this PR can be integrated before the source changes for JEP
> 411, as the possible values of this system property was already defined long
> time ago in JDK 9.
>
> Most of the change in this PR is a simple adding of
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we
> add the line.
>
> There are several tests that launch another Java process that needs to call
> the `System.setSecurityManager()` method, and the system property is added to
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a
> shell test).
>
> 3 langtools tests are added into problem list due to
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>
> 2 SQL tests are moved because they need different options on the `@run` line
> but they are inside a directory that has a `TEST.properties`:
>
> rename test/jdk/java/sql/{testng/test/sql/othervm =>
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi =>
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
> ```
>
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
This pull request has now been integrated.
Changeset: 640a2afd
Author:Weijun Wang
URL:
https://git.openjdk.java.net/jdk/commit/640a2afda36857410b7abf398af81e35430a62e7
Stats: 1028 lines in 852 files changed: 252 ins; 9 del; 767 mod
8267184: Add -Djava.security.manager=allow to tests calling
System.setSecurityManager
Co-authored-by: Lance Andersen
Co-authored-by: Weijun Wang
Reviewed-by: dholmes, alanb, dfuchs, mchung, mullan, prr
-
PR: https://git.openjdk.java.net/jdk/pull/4071
Re: RFR: 8133686: HttpURLConnection.getHeaderFields and URLConnection.getRequestProperties methods return field values in reverse order [v3]
> Hi all, > > Please review this fix for which corrects the order in which field values are > returned from the `HttpURLConnection.getHeaderFields` and > `URLConnection.getRequestProperties` methods. > > Currently, the implementation of these methods returns the values in reverse. > This does not conform with the RFC2616 spec which outlines that the order of > these field values should not be changed. > > Thanks, > Evan Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: MessageHeaderTest add comma to copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/2294/files - new: https://git.openjdk.java.net/jdk/pull/2294/files/1a74ae6e..4e0c96cb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2294&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2294&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2294.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2294/head:pull/2294 PR: https://git.openjdk.java.net/jdk/pull/2294
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
> On May 24, 2021, at 10:39 AM, Mark Sheppard wrote: > > I could have used return directly in multiple places ... but my style > preference is a single exit point from a method My preference is the opposite :-) I like the "early returns" coding style because I don't need to "keep state" while reviewing code; specially long methods (my brain is not as good as it used to be). In this case there is also over initialization of "s", (it does not need to be set to null), and because of the java flow control analysis deficiencies "s" cannot be declared to be "final". So getting rid of the variable makes a lot of sense to me (fewer lines of code and less complexity). Best, christos signature.asc Description: Message signed with OpenPGP
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
On Mon, 24 May 2021 12:30:38 GMT, Mark Sheppard wrote: >> The test java/net/Socket/UdpSocket.java has been seen to fail with a >> BindException, in the testMaxSockets test, on a regular basis on >> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP >> Sockets that may be created as defined by a system property >> sun.net.maxDatagramSockets. It invokes the Socket constructor >> Socket(InetAddress host, int port, boolean stream) with stream set to false >> to create a UDP Socket. This instantiation is a compound operation, >> consisting of the creation of a socket, the explicit binding of wildcard >> address and ephemeral port, and a connect to the socket end point specified >> in the constructor parameters. Analysis has shown that during the test that >> the OS intermittently allocates the same ephemeral port multiple times >> during the bind system call, such that two separate sockets end up bound to >> the same port. Then on the connect invocation a BindException is thrown for >> the second socket. This has been determined to be a transient condition dur ing heavy loads and where a significant number of ephemeral ports are being allocated. >> >> As this is deemed an OS issues, and in order to increase test stability, it >> has been found that for the BindException condition a retry of the Socket >> creation mitigates the issues and tests the max creation property. > > Mark Sheppard has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8265362 java/net/Socket/UdpSocket.java fails with > "java.net.BindException: Address already in use" (macos-aarch64) > update as per request by AB Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
On Mon, 24 May 2021 14:34:54 GMT, Mark Sheppard wrote: >> Mark Sheppard has updated the pull request incrementally with one additional >> commit since the last revision: >> >> JDK-8265362 java/net/Socket/UdpSocket.java fails with >> "java.net.BindException: Address already in use" (macos-aarch64) >> update as per request by AB > > I used stdout for convenience as it aligns with the two other output > statements, so that I see immediately the retry output as follows: > > --System.out:(12/349)-- > [TestNG] Running: > java/net/Socket/UdpSocket.java > > BindException caught retry Socket creation > test UdpSocket.testMaxSockets(): success > test UdpSocket.testSendReceive(): success > > === I could have used return directly in multiple places ... but my style preference is a single exit point from a method - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
On Mon, 24 May 2021 12:30:38 GMT, Mark Sheppard wrote: >> The test java/net/Socket/UdpSocket.java has been seen to fail with a >> BindException, in the testMaxSockets test, on a regular basis on >> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP >> Sockets that may be created as defined by a system property >> sun.net.maxDatagramSockets. It invokes the Socket constructor >> Socket(InetAddress host, int port, boolean stream) with stream set to false >> to create a UDP Socket. This instantiation is a compound operation, >> consisting of the creation of a socket, the explicit binding of wildcard >> address and ephemeral port, and a connect to the socket end point specified >> in the constructor parameters. Analysis has shown that during the test that >> the OS intermittently allocates the same ephemeral port multiple times >> during the bind system call, such that two separate sockets end up bound to >> the same port. Then on the connect invocation a BindException is thrown for >> the second socket. This has been determined to be a transient condition dur ing heavy loads and where a significant number of ephemeral ports are being allocated. >> >> As this is deemed an OS issues, and in order to increase test stability, it >> has been found that for the BindException condition a retry of the Socket >> creation mitigates the issues and tests the max creation property. > > Mark Sheppard has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8265362 java/net/Socket/UdpSocket.java fails with > "java.net.BindException: Address already in use" (macos-aarch64) > update as per request by AB I used stdout for convenience as it aligns with the two other output statements, so that I see immediately the retry output as follows: --System.out:(12/349)-- [TestNG] Running: java/net/Socket/UdpSocket.java BindException caught retry Socket creation test UdpSocket.testMaxSockets(): success test UdpSocket.testSendReceive(): success === - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
On Mon, 24 May 2021 12:30:38 GMT, Mark Sheppard wrote: >> The test java/net/Socket/UdpSocket.java has been seen to fail with a >> BindException, in the testMaxSockets test, on a regular basis on >> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP >> Sockets that may be created as defined by a system property >> sun.net.maxDatagramSockets. It invokes the Socket constructor >> Socket(InetAddress host, int port, boolean stream) with stream set to false >> to create a UDP Socket. This instantiation is a compound operation, >> consisting of the creation of a socket, the explicit binding of wildcard >> address and ephemeral port, and a connect to the socket end point specified >> in the constructor parameters. Analysis has shown that during the test that >> the OS intermittently allocates the same ephemeral port multiple times >> during the bind system call, such that two separate sockets end up bound to >> the same port. Then on the connect invocation a BindException is thrown for >> the second socket. This has been determined to be a transient condition dur ing heavy loads and where a significant number of ephemeral ports are being allocated. >> >> As this is deemed an OS issues, and in order to increase test stability, it >> has been found that for the BindException condition a retry of the Socket >> creation mitigates the issues and tests the max creation property. > > Mark Sheppard has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8265362 java/net/Socket/UdpSocket.java fails with > "java.net.BindException: Address already in use" (macos-aarch64) > update as per request by AB Changes requested by zoul...@github.com (no known OpenJDK username). - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v4]
> Please review the test changes for [JEP
> 411](https://openjdk.java.net/jeps/411).
>
> With JEP 411 and the default value of `-Djava.security.manager` becoming
> `disallow`, tests calling `System.setSecurityManager()` need
> `-Djava.security.manager=allow` when launched. This PR covers such changes
> for tier1 to tier3 (except for the JCK tests).
>
> To make it easier to focus your review on the tests in your area, this PR is
> divided into multiple commits for different areas (~~serviceability~~,
> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d,
> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~,
> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). Mostly the rule is the
> same as how Skara adds labels, but there are several small tweaks:
>
> 1. When a file is covered by multiple labels, only one is chosen. I make my
> best to avoid putting too many tests into `core-libs`. If a file is covered
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the
> `rmi` commit.
> 4. One file not covered by any label --
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in
> the `swing` commit.
>
> Due to the size of this PR, no attempt is made to update copyright years for
> all files to minimize unnecessary merge conflict.
>
> Please note that this PR can be integrated before the source changes for JEP
> 411, as the possible values of this system property was already defined long
> time ago in JDK 9.
>
> Most of the change in this PR is a simple adding of
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we
> add the line.
>
> There are several tests that launch another Java process that needs to call
> the `System.setSecurityManager()` method, and the system property is added to
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a
> shell test).
>
> 3 langtools tests are added into problem list due to
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>
> 2 SQL tests are moved because they need different options on the `@run` line
> but they are inside a directory that has a `TEST.properties`:
>
> rename test/jdk/java/sql/{testng/test/sql/othervm =>
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi =>
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
> ```
>
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
Weijun Wang has updated the pull request with a new target base due to a merge
or a rebase. The incremental webrev excludes the unrelated changes brought in
by the merge/rebase. The pull request contains 20 additional commits since the
last revision:
- Merge branch 'master' into 8267184
- feedback from Phil
reverted:
- adjust order of VM options
- test for awt
- test for hotspot-gc
- test for compiler
- test for net
- test for core-libs
- test for beans
- test for xml
- ... and 10 more: https://git.openjdk.java.net/jdk/compare/37f74de7...412264a0
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4071/files
- new: https://git.openjdk.java.net/jdk/pull/4071/files/9a3ec578..412264a0
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=02-03
Stats: 12227 lines in 453 files changed: 6978 ins; 3721 del; 1528 mod
Patch: https://git.openjdk.java.net/jdk/pull/4071.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071
PR: https://git.openjdk.java.net/jdk/pull/4071
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v4]
On Sun, 23 May 2021 16:35:43 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/java/lang/SecurityManager.java line 104:
>>
>>> 102: * method will throw an {@code UnsupportedOperationException}). If the
>>> 103: * {@systemProperty java.security.manager} system property is set to
>>> the
>>> 104: * special token "{@code allow}", then a security manager will not be
>>> set at
>>
>> Can/should the `{@systemProperty ...}` tag be used more than once for a
>> given system property? I thought it should be used only once, at the place
>> where the system property is defined. Maybe @jonathan-gibbons can offer some
>> more guidance on this.
>
> Good point. I would remove the extra @systemProperty tags on lines 103, 106,
> and 113. Also, in `System.setSecurityManager` there are 3 @systemProperty
> java.security.manager tags, so we should remove those too.
New commit pushed. There is only one `@systemProperty` tag now.
-
PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v4]
> Please review this implementation of [JEP > 411](https://openjdk.java.net/jeps/411). > > The code change is divided into 3 commits. Please review them one by one. > > 1. > https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 > The essential change for this JEP, including the `@Deprecate` annotations > and spec change. It also update the default value of the > `java.security.manager` system property to "disallow", and necessary test > change following this update. > 2. > https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 > Manual changes to several files so that the next commit can be generated > programatically. > 3. > https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 > Automatic changes to other source files to avoid javac warnings on > deprecation for removal > > The 1st and 2nd commits should be reviewed carefully. The 3rd one is > generated programmatically, see the comment below for more details. If you > are only interested in a portion of the 3rd commit and would like to review > it as a separate file, please comment here and I'll generate an individual > webrev. > > Due to the size of this PR, no attempt is made to update copyright years for > any file to minimize unnecessary merge conflict. > > Furthermore, since the default value of `java.security.manager` system > property is now "disallow", most of the tests calling > `System.setSecurityManager()` need to launched with > `-Djava.security.manager=allow`. This is covered in a different PR at > https://github.com/openjdk/jdk/pull/4071. > > Update: the deprecation annotations and javadoc tags, build, compiler, > core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are > reviewed. Rest are 2d, awt, beans, sound, and swing. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: keep only one systemProperty tag - Changes: - all: https://git.openjdk.java.net/jdk/pull/4073/files - new: https://git.openjdk.java.net/jdk/pull/4073/files/c4221b5f..1f6ff6c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=02-03 Stats: 8 lines in 2 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4073.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073 PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
On Sat, 22 May 2021 10:54:06 GMT, Mark Sheppard wrote: >> BTW: Is one retry enough? There is at least one other replace where we've >> had to retry to workaround a macOS bug and one retry was enough there too. > > I have submitted a significant number of MACH5 job runs with repeat mode over > the past week - approx 50 x 50, and observed the retry to have been > triggered about 5 times, this gives some level of assurance that test > stability will exist. But CI builds will exercise this a lot more. updated as requested newUdpSocket --> s, and biEx --> unexpected - PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]
> The test java/net/Socket/UdpSocket.java has been seen to fail with a > BindException, in the testMaxSockets test, on a regular basis on > macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP > Sockets that may be created as defined by a system property > sun.net.maxDatagramSockets. It invokes the Socket constructor > Socket(InetAddress host, int port, boolean stream) with stream set to false > to create a UDP Socket. This instantiation is a compound operation, > consisting of the creation of a socket, the explicit binding of wildcard > address and ephemeral port, and a connect to the socket end point specified > in the constructor parameters. Analysis has shown that during the test that > the OS intermittently allocates the same ephemeral port multiple times during > the bind system call, such that two separate sockets end up bound to the same > port. Then on the connect invocation a BindException is thrown for the second > socket. This has been determined to be a transient condition duri ng heavy loads and where a significant number of ephemeral ports are being allocated. > > As this is deemed an OS issues, and in order to increase test stability, it > has been found that for the BindException condition a retry of the Socket > creation mitigates the issues and tests the max creation property. Mark Sheppard has updated the pull request incrementally with one additional commit since the last revision: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) update as per request by AB - Changes: - all: https://git.openjdk.java.net/jdk/pull/4103/files - new: https://git.openjdk.java.net/jdk/pull/4103/files/1f05203e..87cd9c5e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4103&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4103&range=00-01 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4103.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4103/head:pull/4103 PR: https://git.openjdk.java.net/jdk/pull/4103
Re: RFR: 8263561: Re-examine uses of LinkedList
On Mon, 24 May 2021 10:13:55 GMT, Сергей Цыпанов wrote: >> But don't people access these internal code at their own risk, as jdk may >> change these code at any time without notice? > > True, I just wonder whether it's OK to change internals when we know for sure > that this breaks 3rd party code I think so. There are always unexpected ways the jdk may break and third-party libraries would need a different workaround for a new java version. For instance, in Apache log4j, its library has a special guard against the broken implementation of Reflection getCallerClass during java 7. The libraries can just handle these in their version-specific components. Especially given the fact that the code linked above already has version-specific handling (8 vs 9), so it won't hurt much for them to add another piece of logic to handle jdk 17+ in case this optimization is merged. - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Mon, 24 May 2021 09:25:08 GMT, liach wrote: >> Should we then revert the changes to `JarIndex`? > > But don't people access these internal code at their own risk, as jdk may > change these code at any time without notice? True, I just wonder whether it's OK to change internals when we know for sure that this breaks 3rd party code - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Mon, 24 May 2021 07:13:29 GMT, Сергей Цыпанов wrote: >> Just for completeness, they're using the add-exports: >> https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/launchers/itw-modularjdk.args#L19 > > Should we then revert the changes to `JarIndex`? But don't people access these internal code at their own risk, as jdk may change these code at any time without notice? - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]
On Fri, 21 May 2021 20:37:30 GMT, Weijun Wang wrote:
>> The code change refactors classes that have a `SuppressWarnings("removal")`
>> annotation that covers more than 50KB of code. The big annotation is often
>> quite faraway from the actual deprecated API usage it is suppressing, and
>> with the annotation covering too big a portion it's easy to call other
>> deprecated methods without being noticed.
>>
>> The code change shows some common solutions to avoid such too wide
>> annotations:
>>
>> 1. Extract calls into a method and add annotation on that method
>> 2. Assign the return value of a deprecated method call to a new local
>> variable and add annotation on the declaration, and then assign that value
>> to the original l-value.
>> 3. Put declaration and assignment into a single statement if possible.
>> 4. Rewrite code to achieve #3 above.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update FtpClient.java
Thanks for taking in my suggestions for FtpClient. I have also reviewed the
changes to java.util.logging and JMX. I wish there was a way to handle
doPrivileged returning void more nicely. Maybe component maintainers will be
able to deal with them on a case-by-case basis later on.
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4138
Re: RFR: 8263561: Re-examine uses of LinkedList
On Fri, 21 May 2021 15:12:20 GMT, Thiago Henrique Hüpner wrote: >>> IcedTea-Web seems to be using this method reflectively: >>> https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/common/src/main/java/net/adoptopenjdk/icedteaweb/jdk89access/JarIndexAccess.java#L80 >> >> I assume this doesn't work with JDK 16, at least not without using >> --add-exports to export jdk.internal.util.jar. > > Just for completeness, they're using the add-exports: > https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/launchers/itw-modularjdk.args#L19 Should we then revert the changes to `JarIndex`? - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Fri, 21 May 2021 14:18:16 GMT, Daniel Fuchs wrote: >> The usage of `LinkedList` is senseless and can be replaced with either >> `ArrayList` or `ArrayDeque` which are both more compact and effective. >> >> jdk:tier1 and jdk:tier2 are both ok > > src/java.base/share/classes/jdk/internal/util/jar/JarIndex.java line 264: > >> 262: String jar = jarFiles[i]; >> 263: bw.write(jar + "\n"); >> 264: ArrayList jarlist = jarMap.get(jar); > > Here again, jarList could probably be declared as `List` Done - PR: https://git.openjdk.java.net/jdk/pull/2744
Re: RFR: 8263561: Re-examine uses of LinkedList
On Fri, 21 May 2021 14:15:53 GMT, Daniel Fuchs wrote:
>> The usage of `LinkedList` is senseless and can be replaced with either
>> `ArrayList` or `ArrayDeque` which are both more compact and effective.
>>
>> jdk:tier1 and jdk:tier2 are both ok
>
> src/java.base/share/classes/jdk/internal/util/jar/JarIndex.java line 155:
>
>> 153: */
>> 154: public List get(String fileName) {
>> 155: ArrayList jarFiles;
>
> This could probably be declared as:
>
>
> List jarFiles;
Done
-
PR: https://git.openjdk.java.net/jdk/pull/2744
