On Tue, 26 Nov 2024 09:07:33 GMT, Volkan Yazıcı <d...@openjdk.org> wrote:
>> Volkan Yazıcı has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 23 commits: >> >> - Merge remote-tracking branch 'upstream/master' into >> Socket-CloseOnFailure-8343791 >> - Add tests using real sockets >> - Add tests with real sockets >> - Relax assertion to fix failing `IDNTest` >> - Merge remote-tracking branch 'upstream/master' into >> Socket-CloseOnFailure-8343791 >> - Reflect behavioral changes to `SocketAdaptor` >> - Improve variable names >> - Improve naming in tests >> - Handle `SocketTimeoutException` in `NioSocketImpl::connect()` >> - Revert `UHE` message change in `NioSocketImpl` >> - ... and 13 more: https://git.openjdk.org/jdk/compare/9793e73b...e0fb7061 > > test/jdk/java/net/Socket/CloseOnFailureTest.java line 345: > >> 343: SocketAddress serverSocketAddress = >> serverSocket().getLocalSocketAddress(); >> 344: try (Socket socket = >> createConnectedSocket(serverSocketAddress)) { >> 345: assertReconnectFailure(() -> >> socket.connect(REFUSING_SOCKET_ADDRESS)); > > I needed to delegate the assertion to the implementation because when I call > `connect()` on a `Socket` created using > `SocketChannel.open(address).socket()` and `new Socket(address, port)`, the > former throws an `AlreadyConnectedException`, whereas the latter throws a > `SocketException`. Is this discrepancy expected? There are some subtle behaviour discrepancies between plain Socket (created with new Socket()) and the SocketAdaptor (created from SocketChannel.socket()). SocketChannel::connect will throw AlreadyConnectedException, and so that's what the socket adaptor also does. Ideally we'd want to reduce the distance between plain Socket and the Socket adaptor - but changing behaviours in these APIs is always tricky and a bit risky (and can require a lot of efforts). FWIW - SocketChannel does the right thing - but we're stuck with SocketException in the Socket API due to compatibility reason. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1858226183
