RFR: 8235139: Remove the socket impl factory mechanism
Hi, Could someone please review my proposed changeset for JDK-8235139: '`Remove the socket impl factory mechanism`' ? These changes propose to deprecate (for the eventual removal) the API points for statically configuring a system-wide factory for the `Socket`, `ServerSocket`, and `DatagramSocket` types in the `java.net package`. Specifically, the following: **Methods**: - `static void Socket.setSocketImplFactory(SocketImplFactory fac)` - `static void ServerSocket.setSocketFactory(SocketImplFactory fac)` - `static void DatagramSocket.setDatagramSocketImplFactory(DatagramSocketImplFactory fac)` **Types**: - `java.net SocketImplFactory` - `java.net DatagramSocketImplFactory` The CSR for[ JDK-8220494](https://bugs.openjdk.java.net/browse/JDK-8220494) contains some verbiage about the potential issues that setting factories can have, and alludes to their possible future removal. Kind regards, Patrick - Commit messages: - 8235139: Remove the socket impl factory mechanism Changes: https://git.openjdk.java.net/jdk/pull/2375/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2375&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8235139 Stats: 53 lines in 3 files changed: 26 ins; 22 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/2375.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2375/head:pull/2375 PR: https://git.openjdk.java.net/jdk/pull/2375
Re: RFR: 8235139: Remove the socket impl factory mechanism
On Wed, 3 Feb 2021 11:03:51 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my proposed changeset for JDK-8235139: '`Remove
> the socket impl factory mechanism`' ?
>
> These changes propose to deprecate (for the eventual removal) the API points
> for statically configuring a system-wide factory for the `Socket`,
> `ServerSocket`, and `DatagramSocket` types in the `java.net package`.
> Specifically, the following:
>
> **Methods**:
> - `static void Socket.setSocketImplFactory(SocketImplFactory fac)`
> - `static void ServerSocket.setSocketFactory(SocketImplFactory fac)`
> - `static void
> DatagramSocket.setDatagramSocketImplFactory(DatagramSocketImplFactory fac)`
>
> **Types**:
> - `java.net SocketImplFactory`
> - `java.net DatagramSocketImplFactory`
>
> The CSR for[ JDK-8220494](https://bugs.openjdk.java.net/browse/JDK-8220494)
> contains some verbiage about the potential
> issues that setting factories can have, and alludes to their possible
> future removal.
>
> Kind regards,
> Patrick
Changes requested by dfuchs (Reviewer).
src/java.base/share/classes/java/net/Socket.java line 546:
> 544: @SuppressWarnings("deprecation")
> 545: void setImpl() {
> 546: SocketImplFactory factory = Socket.factory;
The scope of `@SuppressWarnings` should be reduced as much as possible. I
believe in this case it can be reduced to the assignation of the factory:
@SuppressWarnings("deprecation")
SocketImplFactory factory = Socket.factory;
-
PR: https://git.openjdk.java.net/jdk/pull/2375
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification
On Tue, 2 Feb 2021 16:14:10 GMT, Michael McMahon wrote: >> This change clarifies the InetSocketAddress::toString specification, which >> was recently updated to reflect an implementation change [1]. The >> specification is not changed, but it is enhanced with two examples and some >> more verbiage, as per earlier suggestions on the net-dev mailing list [2][3]. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8225499 >> [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html >> [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html > > Maybe could say "rather than parsing the output of toString()" instead of > "rather than parsing the string representation". Might be clearer? Ok, how about this: "rather than parsing the string returned by this toString() method"? - PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: 8235139: Remove the socket impl factory mechanism
On Wed, 3 Feb 2021 11:36:23 GMT, Daniel Fuchs wrote: >> Hi, >> >> Could someone please review my proposed changeset for JDK-8235139: '`Remove >> the socket impl factory mechanism`' ? >> >> These changes propose to deprecate (for the eventual removal) the API points >> for statically configuring a system-wide factory for the `Socket`, >> `ServerSocket`, and `DatagramSocket` types in the `java.net package`. >> Specifically, the following: >> >> **Methods**: >> - `static void Socket.setSocketImplFactory(SocketImplFactory fac)` >> - `static void ServerSocket.setSocketFactory(SocketImplFactory fac)` >> - `static void >> DatagramSocket.setDatagramSocketImplFactory(DatagramSocketImplFactory fac)` >> >> **Types**: >> - `java.net SocketImplFactory` >> - `java.net DatagramSocketImplFactory` >> >> The CSR for[ JDK-8220494](https://bugs.openjdk.java.net/browse/JDK-8220494) >> contains some verbiage about the potential >> issues that setting factories can have, and alludes to their possible >> future removal. >> >> Kind regards, >> Patrick > > Changes requested by dfuchs (Reviewer). The title of the JBS issue should probably be renamed to "Deprecate the socket impl factory mechanism"... Maybe wait on further feedback before actually making this change. - PR: https://git.openjdk.java.net/jdk/pull/2375
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification
On Wed, 3 Feb 2021 11:36:28 GMT, Julia Boes wrote: >> Maybe could say "rather than parsing the output of toString()" instead of >> "rather than parsing the string representation". Might be clearer? > > Ok, how about this: "rather than parsing the string returned by this > toString() method"? Thank you for this change. This PR looks fine to me and your latest suggestion about rewording that last paragraph sounds good too. - PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification
On Tue, 2 Feb 2021 15:55:40 GMT, Julia Boes wrote: > This change clarifies the InetSocketAddress::toString specification, which > was recently updated to reflect an implementation change [1]. The > specification is not changed, but it is enhanced with two examples and some > more verbiage, as per earlier suggestions on the net-dev mailing list [2][3]. > > [1] https://bugs.openjdk.java.net/browse/JDK-8225499 > [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html > [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification
On Tue, 2 Feb 2021 15:55:40 GMT, Julia Boes wrote: > This change clarifies the InetSocketAddress::toString specification, which > was recently updated to reflect an implementation change [1]. The > specification is not changed, but it is enhanced with two examples and some > more verbiage, as per earlier suggestions on the net-dev mailing list [2][3]. > > [1] https://bugs.openjdk.java.net/browse/JDK-8225499 > [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html > [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html Marked as reviewed by michaelm (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2357
Integrated: JDK-8241995: Clarify InetSocketAddress::toString specification
On Tue, 2 Feb 2021 15:55:40 GMT, Julia Boes wrote: > This change clarifies the InetSocketAddress::toString specification, which > was recently updated to reflect an implementation change [1]. The > specification is not changed, but it is enhanced with two examples and some > more verbiage, as per earlier suggestions on the net-dev mailing list [2][3]. > > [1] https://bugs.openjdk.java.net/browse/JDK-8225499 > [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html > [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html This pull request has now been integrated. Changeset: b0ee7a86 Author:Julia Boes URL: https://git.openjdk.java.net/jdk/commit/b0ee7a86 Stats: 12 lines in 1 file changed: 7 ins; 0 del; 5 mod 8241995: Clarify InetSocketAddress::toString specification Reviewed-by: michaelm, chegar - PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification [v2]
> This change clarifies the InetSocketAddress::toString specification, which > was recently updated to reflect an implementation change [1]. The > specification is not changed, but it is enhanced with two examples and some > more verbiage, as per earlier suggestions on the net-dev mailing list [2][3]. > > [1] https://bugs.openjdk.java.net/browse/JDK-8225499 > [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html > [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html Julia Boes has updated the pull request incrementally with one additional commit since the last revision: small adjustment - Changes: - all: https://git.openjdk.java.net/jdk/pull/2357/files - new: https://git.openjdk.java.net/jdk/pull/2357/files/f3394178..f022f7e2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2357&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2357&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2357.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2357/head:pull/2357 PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification [v2]
On Wed, 3 Feb 2021 14:17:58 GMT, Julia Boes wrote:
>> This change clarifies the InetSocketAddress::toString specification, which
>> was recently updated to reflect an implementation change [1]. The
>> specification is not changed, but it is enhanced with two examples and some
>> more verbiage, as per earlier suggestions on the net-dev mailing list [2][3].
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8225499
>> [2] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013776.html
>> [3] https://mail.openjdk.java.net/pipermail/net-dev/2021-February/015297.html
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> small adjustment
src/java.base/share/classes/java/net/InetSocketAddress.java line 387:
> 385: * To retrieve a string representation of the hostname or the
> address, use
> 386: * {@link #getHostString()}, rather than parsing the string returned
> by this
> 387: * {@link #toString()} method.
A couple of small suggestions:
- "This string" -> "The string"
- For the IPv6 literal case it would good to make it clear that it is enclosed
in square brackets before append the colon and port. It might also be better to
not have this in a separate paragraph because it follows immediately from the
previous sentence.
- The final sentence "To retrieve ..." might be better starting with "Use
getHostString to get the String representation ..."
-
PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: 8235139: Remove the socket impl factory mechanism
On Wed, 3 Feb 2021 11:03:51 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my proposed changeset for JDK-8235139: '`Remove
> the socket impl factory mechanism`' ?
>
> These changes propose to deprecate (for the eventual removal) the API points
> for statically configuring a system-wide factory for the `Socket`,
> `ServerSocket`, and `DatagramSocket` types in the `java.net package`.
> Specifically, the following:
>
> **Methods**:
> - `static void Socket.setSocketImplFactory(SocketImplFactory fac)`
> - `static void ServerSocket.setSocketFactory(SocketImplFactory fac)`
> - `static void
> DatagramSocket.setDatagramSocketImplFactory(DatagramSocketImplFactory fac)`
>
> **Types**:
> - `java.net SocketImplFactory`
> - `java.net DatagramSocketImplFactory`
>
> The CSR for[ JDK-8220494](https://bugs.openjdk.java.net/browse/JDK-8220494)
> contains some verbiage about the potential
> issues that setting factories can have, and alludes to their possible
> future removal.
>
> Kind regards,
> Patrick
src/java.base/share/classes/java/net/DatagramSocket.java line 959:
> 957: *and using the {@linkplain #DatagramSocket(DatagramSocketImpl)
> protected
> 958: *constructor} that takes an {@linkplain DatagramSocketImpl
> implementation}
> 959: *as a parameter.
This wording is good, are you planning to add this to the deprecated methods in
Server and ServerSocket too?
-
PR: https://git.openjdk.java.net/jdk/pull/2375
Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification [v2]
On Wed, 3 Feb 2021 14:19:27 GMT, Alan Bateman wrote:
>> Julia Boes has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> small adjustment
>
> src/java.base/share/classes/java/net/InetSocketAddress.java line 387:
>
>> 385: * To retrieve a string representation of the hostname or the
>> address, use
>> 386: * {@link #getHostString()}, rather than parsing the string
>> returned by this
>> 387: * {@link #toString()} method.
>
> A couple of small suggestions:
> - "This string" -> "The string"
> - For the IPv6 literal case it would good to make it clear that it is
> enclosed in square brackets before append the colon and port. It might also
> be better to not have this in a separate paragraph because it follows
> immediately from the previous sentence.
> - The final sentence "To retrieve ..." might be better starting with "Use
> getHostString to get the String representation ..."
Agreed offline to not apply these suggestions
-
PR: https://git.openjdk.java.net/jdk/pull/2357
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v9]
On Tue, 2 Feb 2021 01:19:01 GMT, Clive Verghese wrote: >> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears >> to not be fully fixed >> >> This also fixes JDK-8259516: Alerts sent by peer may not be received >> correctly during TLS handshake > > Clive Verghese has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test failures It looks good to me now. I will take care of the closed test update. Please finalize the CSR request before integration. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2057
